Skip to content

Commit 98e1eb4

Browse files
ryansteakleyryansteakley
authored
Support suffix exception (#97)
Issue #, if available: aws-controllers-k8s/community#840 Description of changes: Add support for suffix exception checking. Also added Sagemaker tests for prefix and suffix exceptions. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 0736a4b commit 98e1eb4

File tree

11 files changed

+119
-16
lines changed

11 files changed

+119
-16
lines changed

‎pkg/generate/ack/controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ var (
5555
"ResourceExceptionCode": func(r *ackmodel.CRD, httpStatusCode int) string {
5656
return r.ExceptionCode(httpStatusCode)
5757
},
58-
"GoCodeSetExceptionMessagePrefixCheck": func(r *ackmodel.CRD, httpStatusCode int) string {
59-
return code.CheckExceptionMessagePrefix(r.Config(), r, httpStatusCode)
58+
"GoCodeSetExceptionMessageCheck": func(r *ackmodel.CRD, httpStatusCode int) string {
59+
return code.CheckExceptionMessage(r.Config(), r, httpStatusCode)
6060
},
6161
"GoCodeSetReadOneOutput": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int, performSpecUpdate bool) string {
6262
return code.SetResource(r.Config(), r, ackmodel.OpTypeGet, sourceVarName, targetVarName, indentLevel, performSpecUpdate)

‎pkg/generate/code/check.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,35 @@ import (
2424
"github.com/aws-controllers-k8s/code-generator/pkg/names"
2525
)
2626

27-
// CheckExceptionMessagePrefix returns Go code that contains a condition to
28-
// check if the message_prefix specified for a particular HTTP status code in
27+
// CheckExceptionMessage returns Go code that contains a condition to
28+
// check if the message_prefix/message_suffix specified for a particular HTTP status code in
2929
// generator config is a prefix for the exception message returned by AWS API.
30-
// If message_prefix field was not specified for this HTTP code in generator
30+
// If message_prefix/message_suffix field was not specified for this HTTP code in generator
3131
// config, we return an empty string
3232
//
3333
// Sample Output:
3434
//
3535
// && strings.HasPrefix(awsErr.Message(), "Could not find model")
36-
func CheckExceptionMessagePrefix(
36+
// && strings.HasSuffix(awsErr.Message(), "does not exist.")
37+
func CheckExceptionMessage(
3738
cfg *ackgenconfig.Config,
3839
r *model.CRD,
3940
httpStatusCode int,
4041
) string {
4142
rConfig, ok := cfg.ResourceConfig(r.Names.Original)
4243
if ok && rConfig.Exceptions != nil {
4344
excConfig, ok := rConfig.Exceptions.Errors[httpStatusCode]
44-
if ok && excConfig.MessagePrefix != nil {
45+
if !ok {
46+
return ""
47+
}
48+
if excConfig.MessagePrefix != nil {
4549
return fmt.Sprintf("&& strings.HasPrefix(awsErr.Message(), \"%s\") ",
4650
*excConfig.MessagePrefix)
4751
}
52+
if excConfig.MessageSuffix != nil {
53+
return fmt.Sprintf("&& strings.HasSuffix(awsErr.Message(), \"%s\") ",
54+
*excConfig.MessageSuffix)
55+
}
4856
}
4957
return ""
5058
}

‎pkg/generate/config/resource.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,15 @@ type ErrorConfig struct {
215215
// later is a terminal state.
216216
// In Go SDK terms - awsErr.Message()
217217
MessagePrefix *string `json:"message_prefix,omitempty"`
218+
// MessageSuffix is an optional string field to be checked as suffix of the
219+
// exception message in addition to exception name. This is needed for HTTP codes
220+
// where the exception name alone is not sufficient to determine the type of error.
221+
// Example: SageMaker service throws ValidationException if job does not exist
222+
// as well as if IAM role does not have sufficient permission to fetch the dataset
223+
// For the former controller should proceed with creation of job whereas the
224+
// later is a terminal state.
225+
// In Go SDK terms - awsErr.Message()
226+
MessageSuffix *string `json:"message_suffix,omitempty"`
218227
}
219228

220229
// RenamesConfig contains instructions to the code generator how to rename

‎pkg/generate/crossplane/crossplane.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ var (
4949
"ResourceExceptionCode": func(r *ackmodel.CRD, httpStatusCode int) string {
5050
return r.ExceptionCode(httpStatusCode)
5151
},
52-
"GoCodeSetExceptionMessagePrefixCheck": func(r *ackmodel.CRD, httpStatusCode int) string {
53-
return code.CheckExceptionMessagePrefix(r.Config(), r, httpStatusCode)
52+
"GoCodeSetExceptionMessageCheck": func(r *ackmodel.CRD, httpStatusCode int) string {
53+
return code.CheckExceptionMessage(r.Config(), r, httpStatusCode)
5454
},
5555
"GoCodeSetReadOneOutput": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int, performSpecUpdate bool) string {
5656
return code.SetResource(r.Config(), r, ackmodel.OpTypeGet, sourceVarName, targetVarName, indentLevel, performSpecUpdate)

‎pkg/generate/sagemaker_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/stretchr/testify/assert"
2020
"github.com/stretchr/testify/require"
2121

22+
"github.com/aws-controllers-k8s/code-generator/pkg/generate/code"
2223
"github.com/aws-controllers-k8s/code-generator/pkg/testutil"
2324
)
2425

@@ -66,3 +67,76 @@ func TestSageMaker_ARN_Field_Override(t *testing.T) {
6667
assert.Equal(true, crd.IsPrimaryARNField("JobDefinitionArn"))
6768

6869
}
70+
71+
func TestSageMaker_Error_Prefix_Message(t *testing.T) {
72+
assert := assert.New(t)
73+
require := require.New(t)
74+
75+
g := testutil.NewGeneratorForService(t, "sagemaker")
76+
77+
crds, err := g.GetCRDs()
78+
require.Nil(err)
79+
80+
crd := getCRDByName("TrainingJob", crds)
81+
require.NotNil(crd)
82+
83+
require.NotNil(crd.Ops)
84+
85+
assert.NotNil(crd.Ops.ReadOne)
86+
87+
// "DescribeTrainingJob":{
88+
// "name":"DescribeTrainingJob",
89+
// "http":{
90+
// "method":"POST",
91+
// "requestUri":"/"
92+
// },
93+
// "input":{"shape":"DescribeTrainingJobRequest"},
94+
// "output":{"shape":"DescribeTrainingJobResponse"},
95+
// "errors":[
96+
// {"shape":"ResourceNotFound"}
97+
// ]
98+
// },
99+
100+
// Which does not indicate that the error is a 404 :( So, the logic in the
101+
// CRD.ExceptionCode(404) method needs to get its override from the
102+
// generate.yaml configuration file.
103+
assert.Equal("ValidationException", crd.ExceptionCode(404))
104+
105+
// Validation Exception has prefix Requested resource not found.
106+
assert.Equal("&& strings.HasPrefix(awsErr.Message(), \"Requested resource not found\") ", code.CheckExceptionMessage(crd.Config(), crd, 404))
107+
}
108+
109+
func TestSageMaker_Error_Suffix_Message(t *testing.T) {
110+
assert := assert.New(t)
111+
require := require.New(t)
112+
113+
g := testutil.NewGeneratorForService(t, "sagemaker")
114+
115+
crds, err := g.GetCRDs()
116+
require.Nil(err)
117+
118+
crd := getCRDByName("ModelPackageGroup", crds)
119+
require.NotNil(crd)
120+
121+
require.NotNil(crd.Ops)
122+
assert.NotNil(crd.Ops.ReadOne)
123+
124+
// "DescribeModelPackageGroup":{
125+
// "name":"DescribeModelPackageGroup",
126+
// "http":{
127+
// "method":"POST",
128+
// "requestUri":"/"
129+
// },
130+
// "input":{"shape":"DescribeModelPackageGroupInput"},
131+
// "output":{"shape":"DescribeModelPackageGroupOutput"}
132+
// }
133+
134+
// Does not list an error however a ValidationException can occur
135+
// Which does not indicate that the error is a 404 :( So, the logic in the
136+
// CRD.ExceptionCode(404) method needs to get its override from the
137+
// generate.yaml configuration file.
138+
assert.Equal("ValidationException", crd.ExceptionCode(404))
139+
140+
// Validation Exception has suffix ModelPackageGroup arn:aws:sagemaker:/ does not exist
141+
assert.Equal("&& strings.HasSuffix(awsErr.Message(), \"does not exist.\") ", code.CheckExceptionMessage(crd.Config(), crd, 404))
142+
}

‎pkg/generate/testdata/models/apis/sagemaker/0000-00-00/generator.yaml‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@ resources:
77
fields:
88
JobDefinitionArn:
99
is_arn: true
10+
TrainingJob:
11+
exceptions:
12+
errors:
13+
404:
14+
code: ValidationException
15+
message_prefix: Requested resource not found
16+
ModelPackageGroup:
17+
exceptions:
18+
errors:
19+
404:
20+
code: ValidationException
21+
message_suffix: does not exist.
1022
ignore:
1123
resource_names:
1224
- Algorithm
@@ -36,7 +48,7 @@ ignore:
3648
- ModelBiasJobDefinition
3749
- ModelExplainabilityJobDefinition
3850
- ModelPackage
39-
- ModelPackageGroup
51+
# ModelPackageGroup
4052
- ModelQualityJobDefinition
4153
- MonitoringSchedule
4254
- NotebookInstanceLifecycleConfig
@@ -46,7 +58,7 @@ ignore:
4658
- PresignedNotebookInstanceUrl
4759
- ProcessingJob
4860
- Project
49-
- TrainingJob
61+
# TrainingJob
5062
- TransformJob
5163
- TrialComponent
5264
- Trial

‎templates/crossplane/pkg/conversions.go.tpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@ func Generate{{ .CRD.Ops.Delete.InputRef.Shape.ShapeName }}(cr *svcapitypes.{{ .
5353
// IsNotFound returns whether the given error is of type NotFound or not.
5454
func IsNotFound(err error) bool {
5555
awsErr, ok := err.(awserr.Error)
56-
return ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}
56+
return ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}
5757
}

‎templates/pkg/resource/sdk_find_get_attributes.go.tpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func (rm *resourceManager) sdkFind(
2727
{{- end }}
2828
rm.metrics.RecordAPICall("GET_ATTRIBUTES", "{{ .CRD.Ops.GetAttributes.ExportedName }}", respErr)
2929
if respErr != nil {
30-
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{
30+
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
3131
return nil, ackerr.NotFound
3232
}
3333
return nil, respErr

‎templates/pkg/resource/sdk_find_read_many.go.tpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func (rm *resourceManager) sdkFind(
2020
{{- end }}
2121
rm.metrics.RecordAPICall("READ_MANY", "{{ .CRD.Ops.ReadMany.ExportedName }}", respErr)
2222
if respErr != nil {
23-
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{
23+
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
2424
return nil, ackerr.NotFound
2525
}
2626
return nil, respErr

‎templates/pkg/resource/sdk_find_read_one.go.tpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func (rm *resourceManager) sdkFind(
2727
{{- end }}
2828
rm.metrics.RecordAPICall("READ_ONE", "{{ .CRD.Ops.ReadOne.ExportedName }}", respErr)
2929
if respErr != nil {
30-
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{
30+
if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
3131
return nil, ackerr.NotFound
3232
}
3333
return nil, respErr

0 commit comments

Comments
 (0)