Skip to content

Commit f46cc75

Browse files
nineonineAlex Dudarenko
andauthored
Error message enhancements (#4181)
* Improve CannotPullContainerError message Improve CannotPullContainerError message * Improve CannotPullECRContainerError message (misconfigured network case) * Improve ASM error messages * Code review feedback * CannotPullContainerError: cleanup, renaming * ASM error: do not use added errormessages code - match on AWS SDK error directly, drop tracing prefix, fix/add more unit tests * Cleanup * fix imports * code review - cleanup * code review: renaming suggestion * More review feedback * More review feedback * Finalizing CannotPullContainerError messages * cleanup * Finalizing changes * minor code review feedback * simplify interface * fix unit test --------- Co-authored-by: Alex Dudarenko <[email protected]>
1 parent ffb844c commit f46cc75

File tree

9 files changed

+624
-50
lines changed

9 files changed

+624
-50
lines changed
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
4+
// not use this file except in compliance with the License. A copy of the
5+
// License is located at
6+
//
7+
// http://aws.amazon.com/apache2.0/
8+
//
9+
// or in the "license" file accompanying this file. This file is distributed
10+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
// express or implied. See the License for the specific language governing
12+
// permissions and limitations under the License.
13+
14+
package errormessages
15+
16+
import (
17+
"errors"
18+
"fmt"
19+
"regexp"
20+
21+
apierrors "github.com/aws/amazon-ecs-agent/ecs-agent/api/errors"
22+
"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
23+
)
24+
25+
// This module provides functionality to extend error messages with extra information for
26+
// known Docker client error types. It allows parsing error messages and augmenting them with
27+
// additional details to provide more context to end user. This abstraction simplifies
28+
// error handling and provides richer error messages to aid debugging and troubleshooting.
29+
//
30+
// How it Works:
31+
// The module consists of three main components:
32+
// 1. Error Type Enum: Defines types of Docker errors and associates each error type with a unique identifier.
33+
// 2. Parsers: Functions responsible for parsing error messages and identifying known error types.
34+
// Each parser is associated with a specific error pattern and error type.
35+
// 3. Error Message Formatters: Functions to generate rich error messages for each known error type.
36+
//
37+
// Adding a New Error Type:
38+
// To add a new error type, follow these steps:
39+
// 1. Define a new constant in the DockerErrorType enum for the new error type.
40+
// 2. Implement a parser function to identify the error pattern associated with the new error type.
41+
// Add the parser function in `AugmentMessage`.
42+
// 3. Update `errorParsers` map to associate the new error type with its parser function.
43+
// 4. Implement a formatter function to generate error messages for the new error type. The formatter
44+
// function should take parsed error data and additional arguments as input and produce an augmented
45+
// error message.
46+
// 5. Update `errorMessageFunctions` map to associate the new error type with its formatter function.
47+
//
48+
// Usage:
49+
// To augment an error message, call the AugmentNamedErrMsg or AugmentMessage function with the original error
50+
// If the error message matches known error pattern, it will be augmented with extra information; otherwise,
51+
// the original error message will be returned.
52+
//
53+
// Example:
54+
// augmentedMsg := apierrors.AugmentMessage("API error (404): repository not found")
55+
//
56+
57+
// DockerErrorType represents the type of error returned by Docker client.
58+
type DockerErrorType int
59+
60+
const (
61+
MissingECRBatchGetImageError DockerErrorType = iota
62+
ECRImageDoesNotExistError
63+
NetworkConfigurationError
64+
)
65+
66+
type DockerError struct {
67+
Type DockerErrorType
68+
RawError string
69+
}
70+
71+
// Defines the function signature for generating formatted error messages.
72+
type ErrorMessageFunc func(errorData DockerError) string
73+
74+
// A map associating DockerErrorType with functions to format error messages.
75+
var errorMessageFunctions = map[DockerErrorType]ErrorMessageFunc{
76+
MissingECRBatchGetImageError: formatMissingECRBatchGetImageError,
77+
ECRImageDoesNotExistError: formatImageDoesNotExistError,
78+
NetworkConfigurationError: formatNetworkConfigurationError,
79+
}
80+
81+
// A map associating DockerErrorType with parser functions.
82+
var errorParsers = map[DockerErrorType]func(string) (DockerError, error){
83+
MissingECRBatchGetImageError: parseMissingPullImagePermsError,
84+
ECRImageDoesNotExistError: parseImageDoesNotExistError,
85+
NetworkConfigurationError: parseNetworkConfigurationError,
86+
}
87+
88+
// An interface that provides means to reconstruct Named Errors. This is
89+
// used during error message augmentation process.
90+
type AugmentableNamedError interface {
91+
apierrors.NamedError
92+
WithAugmentedErrorMessage(string) apierrors.NamedError
93+
}
94+
95+
const (
96+
// error message patterns used by parsers
97+
PatternECRBatchGetImageError = `denied: User: (.+) is not authorized to perform: ecr:BatchGetImage on resource`
98+
PatternImageDoesNotExistError = `denied: requested access to the resource is denied`
99+
PatternNetworkConfigurationError = `request canceled while waiting for connection`
100+
// internal errors
101+
ErrorMessageDoesNotMatch = `ErrorMessageDoesNotMatch`
102+
)
103+
104+
// A series of CannotPullContainerError error message parsers. Example messages:
105+
//
106+
// CannotPullContainerError: Error response from daemon: pull access denied for
107+
// 123123123123.dkr.ecr.us-east-1.amazonaws.com/test_image, repository does not
108+
// exist or may require 'docker login': {details}
109+
// where "details" can be:
110+
// * Missing pull image permissions:
111+
// "denied: User: arn:aws:sts::123123123123:assumed-role/BrokenRole/xyz
112+
// is not authorized to perform: ecr:BatchGetImage on resource:
113+
// arn:aws:ecr:region:123123123123:repository/test_image
114+
// because no identity-based policy allows the ecr:BatchGetImage action"
115+
// * Image repository does not exist (or a type in repo URL):
116+
// "denied: requested access to the resource is denied"
117+
func parseMissingPullImagePermsError(err string) (DockerError, error) {
118+
matched, _ := regexp.MatchString(PatternECRBatchGetImageError, err)
119+
if matched {
120+
return DockerError{
121+
Type: MissingECRBatchGetImageError,
122+
RawError: err,
123+
}, nil
124+
}
125+
return DockerError{RawError: err}, errors.New(ErrorMessageDoesNotMatch)
126+
}
127+
128+
func parseImageDoesNotExistError(err string) (DockerError, error) {
129+
matched, _ := regexp.MatchString(PatternImageDoesNotExistError, err)
130+
if matched {
131+
return DockerError{
132+
Type: ECRImageDoesNotExistError,
133+
RawError: err,
134+
}, nil
135+
}
136+
return DockerError{RawError: err}, errors.New(ErrorMessageDoesNotMatch)
137+
}
138+
139+
func parseNetworkConfigurationError(err string) (DockerError, error) {
140+
matched, _ := regexp.MatchString(PatternNetworkConfigurationError, err)
141+
if matched {
142+
return DockerError{
143+
Type: NetworkConfigurationError,
144+
RawError: err,
145+
}, nil
146+
}
147+
return DockerError{RawError: err}, errors.New(ErrorMessageDoesNotMatch)
148+
}
149+
150+
// Extend error with extra useful information. Works with NamedErrors.
151+
func AugmentNamedErrMsg(namedErr apierrors.NamedError) apierrors.NamedError {
152+
augmentableErr, ok := namedErr.(AugmentableNamedError)
153+
if !ok { // If namedErr is not a AugmentableNamedError, return as-is.
154+
return namedErr
155+
}
156+
157+
// Augment the error message.
158+
augmentedMsg := AugmentMessage(namedErr.Error())
159+
// Reconstruct new NamedError.
160+
newError := augmentableErr.WithAugmentedErrorMessage(augmentedMsg)
161+
return newError
162+
}
163+
164+
// Extend error messages with extra useful information.
165+
// Tries to find known cases by matching `errMsg` against implemented parsers for known errors.
166+
// If the error message pattern is unknown, returns the original error message.
167+
//
168+
// Possible failure scenarios:
169+
// 1. Missing parser implementation - this can only occur if a new ErrorType was added incorrectly
170+
// due to implementation oversight.
171+
//
172+
// Currently, the function is set up in a safe manner - all failures are ignored, and
173+
// the original error message string is returned.
174+
func AugmentMessage(errMsg string) string {
175+
var errorData DockerError
176+
var err error
177+
178+
// Try parsing each error type until a match is found.
179+
for _, parser := range errorParsers {
180+
errorData, err = parser(errMsg)
181+
if err == nil {
182+
// Parser found a match, break out of loop
183+
break
184+
}
185+
}
186+
187+
// none of the parsers match - return original untouched error message
188+
if err != nil {
189+
logger.Debug("AugmentMessage: None of the parsers match - return original error message")
190+
return errMsg
191+
}
192+
193+
// lookup corresponding error formatter function. If not found - return original.
194+
formattedErrorMessage, ok := errorMessageFunctions[errorData.Type]
195+
if !ok {
196+
return errMsg
197+
}
198+
199+
return formattedErrorMessage(errorData)
200+
}
201+
202+
// Generates error message for MissingECRBatchGetImage error.
203+
func formatMissingECRBatchGetImageError(errorData DockerError) string {
204+
rawError := errorData.RawError
205+
206+
formattedMessage := fmt.Sprintf(
207+
"The task can’t pull the image. Check that the role has the permissions to pull images from the registry. %s",
208+
rawError)
209+
210+
return formattedMessage
211+
}
212+
213+
// Generates error message for ECRImageDoesNotExist error.
214+
func formatImageDoesNotExistError(errorData DockerError) string {
215+
rawError := errorData.RawError
216+
217+
formattedMessage := fmt.Sprintf(
218+
"The task can’t pull the image. Check whether the image exists. %s",
219+
rawError)
220+
221+
return formattedMessage
222+
}
223+
224+
// Generates error message for network misconfiguration errors.
225+
func formatNetworkConfigurationError(errorData DockerError) string {
226+
rawError := errorData.RawError
227+
228+
formattedMessage := fmt.Sprintf("The task can’t pull the image. Check your network configuration. %s",
229+
rawError)
230+
231+
return formattedMessage
232+
}
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
//go:build unit
2+
// +build unit
3+
4+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
5+
//
6+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
7+
// not use this file except in compliance with the License. A copy of the
8+
// License is located at
9+
//
10+
// http://aws.amazon.com/apache2.0/
11+
//
12+
// or in the "license" file accompanying this file. This file is distributed
13+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
14+
// express or implied. See the License for the specific language governing
15+
// permissions and limitations under the License.
16+
17+
package errormessages
18+
19+
import (
20+
"errors"
21+
"fmt"
22+
"testing"
23+
24+
apierrors "github.com/aws/amazon-ecs-agent/ecs-agent/api/errors"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
type testCaseAugmentMessage struct {
29+
testName string
30+
errMsg string
31+
expectedMsg string
32+
}
33+
34+
func TestAugmentMessage(t *testing.T) {
35+
testCases := []testCaseAugmentMessage{
36+
{
37+
testName: "Successful augmentation - missing pull permissions",
38+
errMsg: "Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action",
39+
expectedMsg: "The task can’t pull the image. Check that the role has the permissions to pull images from the registry. Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action",
40+
},
41+
{
42+
testName: "Successful augmentation - image does not exist",
43+
errMsg: "Error response from daemon: pull access denied for some/nonsense, repository does not exist or may require 'docker login': denied: requested access to the resource is denied",
44+
expectedMsg: "The task can’t pull the image. Check whether the image exists. Error response from daemon: pull access denied for some/nonsense, repository does not exist or may require 'docker login': denied: requested access to the resource is denied",
45+
},
46+
{
47+
testName: "Successful augmentation - network issues",
48+
errMsg: "RequestError: send request failed\ncaused by: Post \"https://api.ecr.us-east-1.amazonaws.com/\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)",
49+
expectedMsg: "The task can’t pull the image. Check your network configuration. RequestError: send request failed\ncaused by: Post \"https://api.ecr.us-east-1.amazonaws.com/\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)",
50+
},
51+
{
52+
testName: "Does not recognize unknown error",
53+
errMsg: "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: unauthorized: incorrect username or password",
54+
expectedMsg: "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: unauthorized: incorrect username or password",
55+
},
56+
{
57+
testName: "empty",
58+
errMsg: "",
59+
expectedMsg: "",
60+
},
61+
}
62+
63+
for _, tc := range testCases {
64+
t.Run(fmt.Sprintf("AugmentMessage %s", tc.testName), func(t *testing.T) {
65+
actualMsg := AugmentMessage(tc.errMsg)
66+
require.Equal(t, tc.expectedMsg, actualMsg)
67+
})
68+
}
69+
}
70+
71+
type testCaseAugmentNamedErrMsg struct {
72+
testName string
73+
errMsg apierrors.NamedError
74+
expectedMsg string
75+
}
76+
77+
type KnownError struct {
78+
FromError error
79+
}
80+
81+
func (err KnownError) Error() string {
82+
return err.FromError.Error()
83+
}
84+
85+
func (err KnownError) ErrorName() string {
86+
return "KnownError"
87+
}
88+
89+
func (err KnownError) WithAugmentedErrorMessage(msg string) apierrors.NamedError {
90+
return KnownError{errors.New(msg)}
91+
}
92+
93+
// does not implement WithAugmentedErrorMessage()
94+
type UnknownError struct {
95+
FromError error
96+
}
97+
98+
func (err UnknownError) Error() string {
99+
return err.FromError.Error()
100+
}
101+
102+
func (err UnknownError) ErrorName() string {
103+
return "UnknownError"
104+
}
105+
106+
func TestAugmentNamedErrMsg(t *testing.T) {
107+
tests := []struct {
108+
name string
109+
err apierrors.NamedError
110+
expectedMsg string
111+
expectedName string
112+
}{
113+
{
114+
name: "Non-augmentable NamedError",
115+
err: UnknownError{FromError: errors.New("some err")},
116+
expectedMsg: "some err",
117+
expectedName: "UnknownError",
118+
},
119+
{
120+
name: "NamedError error message updated with args",
121+
err: KnownError{FromError: errors.New("Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action")},
122+
expectedMsg: "The task can’t pull the image. Check that the role has the permissions to pull images from the registry. Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action",
123+
expectedName: "KnownError",
124+
},
125+
{
126+
name: "NamedError error message updated no args",
127+
err: KnownError{FromError: errors.New("Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action")},
128+
expectedMsg: "The task can’t pull the image. Check that the role has the permissions to pull images from the registry. Error response from daemon: pull access denied for 123123123123.dkr.ecr.us-east-1.amazonaws.com/my_image, repository does not exist or may require 'docker login': denied: User: arn:aws:sts::123123123123:assumed-role/MyBrokenRole/xyz is not authorized to perform: ecr:BatchGetImage on resource: arn:aws:ecr:us-east-1:123123123123:repository/test_image because no identity-based policy allows the ecr:BatchGetImage action",
129+
expectedName: "KnownError",
130+
},
131+
}
132+
133+
for _, tc := range tests {
134+
t.Run(tc.name, func(t *testing.T) {
135+
augmentedErr := AugmentNamedErrMsg(tc.err)
136+
require.Equal(t, augmentedErr.Error(), tc.expectedMsg)
137+
require.Equal(t, augmentedErr.ErrorName(), tc.expectedName)
138+
})
139+
}
140+
}

0 commit comments

Comments
 (0)