From 318990dfcd9cc69c138191607dad866c9b464ea0 Mon Sep 17 00:00:00 2001 From: Steakley Date: Fri, 18 Jun 2021 12:31:03 -0700 Subject: [PATCH] Support suffix exception and add tests --- pkg/generate/ack/controller.go | 4 +- pkg/generate/code/check.go | 18 +++-- pkg/generate/config/resource.go | 9 +++ pkg/generate/crossplane/crossplane.go | 4 +- pkg/generate/sagemaker_test.go | 74 +++++++++++++++++++ .../apis/sagemaker/0000-00-00/generator.yaml | 16 +++- templates/crossplane/pkg/conversions.go.tpl | 2 +- .../resource/sdk_find_get_attributes.go.tpl | 2 +- .../pkg/resource/sdk_find_read_many.go.tpl | 2 +- .../pkg/resource/sdk_find_read_one.go.tpl | 2 +- .../resource/sdk_update_set_attributes.go.tpl | 2 +- 11 files changed, 119 insertions(+), 16 deletions(-) diff --git a/pkg/generate/ack/controller.go b/pkg/generate/ack/controller.go index d7a7a307..ee80ba5c 100644 --- a/pkg/generate/ack/controller.go +++ b/pkg/generate/ack/controller.go @@ -54,8 +54,8 @@ var ( "ResourceExceptionCode": func(r *ackmodel.CRD, httpStatusCode int) string { return r.ExceptionCode(httpStatusCode) }, - "GoCodeSetExceptionMessagePrefixCheck": func(r *ackmodel.CRD, httpStatusCode int) string { - return code.CheckExceptionMessagePrefix(r.Config(), r, httpStatusCode) + "GoCodeSetExceptionMessageCheck": func(r *ackmodel.CRD, httpStatusCode int) string { + return code.CheckExceptionMessage(r.Config(), r, httpStatusCode) }, "GoCodeSetReadOneOutput": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int, performSpecUpdate bool) string { return code.SetResource(r.Config(), r, ackmodel.OpTypeGet, sourceVarName, targetVarName, indentLevel, performSpecUpdate) diff --git a/pkg/generate/code/check.go b/pkg/generate/code/check.go index e2d7866f..eefa7a03 100644 --- a/pkg/generate/code/check.go +++ b/pkg/generate/code/check.go @@ -24,16 +24,17 @@ import ( "github.com/aws-controllers-k8s/code-generator/pkg/names" ) -// CheckExceptionMessagePrefix returns Go code that contains a condition to -// check if the message_prefix specified for a particular HTTP status code in +// CheckExceptionMessage returns Go code that contains a condition to +// check if the message_prefix/message_suffix specified for a particular HTTP status code in // generator config is a prefix for the exception message returned by AWS API. -// If message_prefix field was not specified for this HTTP code in generator +// If message_prefix/message_suffix field was not specified for this HTTP code in generator // config, we return an empty string // // Sample Output: // // && strings.HasPrefix(awsErr.Message(), "Could not find model") -func CheckExceptionMessagePrefix( +// && strings.HasSuffix(awsErr.Message(), "does not exist.") +func CheckExceptionMessage( cfg *ackgenconfig.Config, r *model.CRD, httpStatusCode int, @@ -41,10 +42,17 @@ func CheckExceptionMessagePrefix( rConfig, ok := cfg.ResourceConfig(r.Names.Original) if ok && rConfig.Exceptions != nil { excConfig, ok := rConfig.Exceptions.Errors[httpStatusCode] - if ok && excConfig.MessagePrefix != nil { + if !ok { + return "" + } + if excConfig.MessagePrefix != nil { return fmt.Sprintf("&& strings.HasPrefix(awsErr.Message(), \"%s\") ", *excConfig.MessagePrefix) } + if excConfig.MessageSuffix != nil { + return fmt.Sprintf("&& strings.HasSuffix(awsErr.Message(), \"%s\") ", + *excConfig.MessageSuffix) + } } return "" } diff --git a/pkg/generate/config/resource.go b/pkg/generate/config/resource.go index 791773d0..7b530208 100644 --- a/pkg/generate/config/resource.go +++ b/pkg/generate/config/resource.go @@ -215,6 +215,15 @@ type ErrorConfig struct { // later is a terminal state. // In Go SDK terms - awsErr.Message() MessagePrefix *string `json:"message_prefix,omitempty"` + // MessageSuffix is an optional string field to be checked as suffix of the + // exception message in addition to exception name. This is needed for HTTP codes + // where the exception name alone is not sufficient to determine the type of error. + // Example: SageMaker service throws ValidationException if job does not exist + // as well as if IAM role does not have sufficient permission to fetch the dataset + // For the former controller should proceed with creation of job whereas the + // later is a terminal state. + // In Go SDK terms - awsErr.Message() + MessageSuffix *string `json:"message_suffix,omitempty"` } // RenamesConfig contains instructions to the code generator how to rename diff --git a/pkg/generate/crossplane/crossplane.go b/pkg/generate/crossplane/crossplane.go index 352eb414..e0ea81ab 100644 --- a/pkg/generate/crossplane/crossplane.go +++ b/pkg/generate/crossplane/crossplane.go @@ -49,8 +49,8 @@ var ( "ResourceExceptionCode": func(r *ackmodel.CRD, httpStatusCode int) string { return r.ExceptionCode(httpStatusCode) }, - "GoCodeSetExceptionMessagePrefixCheck": func(r *ackmodel.CRD, httpStatusCode int) string { - return code.CheckExceptionMessagePrefix(r.Config(), r, httpStatusCode) + "GoCodeSetExceptionMessageCheck": func(r *ackmodel.CRD, httpStatusCode int) string { + return code.CheckExceptionMessage(r.Config(), r, httpStatusCode) }, "GoCodeSetReadOneOutput": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int, performSpecUpdate bool) string { return code.SetResource(r.Config(), r, ackmodel.OpTypeGet, sourceVarName, targetVarName, indentLevel, performSpecUpdate) diff --git a/pkg/generate/sagemaker_test.go b/pkg/generate/sagemaker_test.go index cd96ac1a..8c28784f 100644 --- a/pkg/generate/sagemaker_test.go +++ b/pkg/generate/sagemaker_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/aws-controllers-k8s/code-generator/pkg/generate/code" "github.com/aws-controllers-k8s/code-generator/pkg/testutil" ) @@ -66,3 +67,76 @@ func TestSageMaker_ARN_Field_Override(t *testing.T) { assert.Equal(true, crd.IsPrimaryARNField("JobDefinitionArn")) } + +func TestSageMaker_Error_Prefix_Message(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + g := testutil.NewGeneratorForService(t, "sagemaker") + + crds, err := g.GetCRDs() + require.Nil(err) + + crd := getCRDByName("TrainingJob", crds) + require.NotNil(crd) + + require.NotNil(crd.Ops) + + assert.NotNil(crd.Ops.ReadOne) + + // "DescribeTrainingJob":{ + // "name":"DescribeTrainingJob", + // "http":{ + // "method":"POST", + // "requestUri":"/" + // }, + // "input":{"shape":"DescribeTrainingJobRequest"}, + // "output":{"shape":"DescribeTrainingJobResponse"}, + // "errors":[ + // {"shape":"ResourceNotFound"} + // ] + // }, + + // Which does not indicate that the error is a 404 :( So, the logic in the + // CRD.ExceptionCode(404) method needs to get its override from the + // generate.yaml configuration file. + assert.Equal("ValidationException", crd.ExceptionCode(404)) + + // Validation Exception has prefix Requested resource not found. + assert.Equal("&& strings.HasPrefix(awsErr.Message(), \"Requested resource not found\") ", code.CheckExceptionMessage(crd.Config(), crd, 404)) +} + +func TestSageMaker_Error_Suffix_Message(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + g := testutil.NewGeneratorForService(t, "sagemaker") + + crds, err := g.GetCRDs() + require.Nil(err) + + crd := getCRDByName("ModelPackageGroup", crds) + require.NotNil(crd) + + require.NotNil(crd.Ops) + assert.NotNil(crd.Ops.ReadOne) + + // "DescribeModelPackageGroup":{ + // "name":"DescribeModelPackageGroup", + // "http":{ + // "method":"POST", + // "requestUri":"/" + // }, + // "input":{"shape":"DescribeModelPackageGroupInput"}, + // "output":{"shape":"DescribeModelPackageGroupOutput"} + // } + + // Does not list an error however a ValidationException can occur + // Which does not indicate that the error is a 404 :( So, the logic in the + // CRD.ExceptionCode(404) method needs to get its override from the + // generate.yaml configuration file. + assert.Equal("ValidationException", crd.ExceptionCode(404)) + + // Validation Exception has suffix ModelPackageGroup arn:aws:sagemaker:/ does not exist + assert.Equal("&& strings.HasSuffix(awsErr.Message(), \"does not exist.\") ", code.CheckExceptionMessage(crd.Config(), crd, 404)) +} diff --git a/pkg/generate/testdata/models/apis/sagemaker/0000-00-00/generator.yaml b/pkg/generate/testdata/models/apis/sagemaker/0000-00-00/generator.yaml index 950e2c97..14639618 100644 --- a/pkg/generate/testdata/models/apis/sagemaker/0000-00-00/generator.yaml +++ b/pkg/generate/testdata/models/apis/sagemaker/0000-00-00/generator.yaml @@ -7,6 +7,18 @@ resources: fields: JobDefinitionArn: is_arn: true + TrainingJob: + exceptions: + errors: + 404: + code: ValidationException + message_prefix: Requested resource not found + ModelPackageGroup: + exceptions: + errors: + 404: + code: ValidationException + message_suffix: does not exist. ignore: resource_names: - Algorithm @@ -36,7 +48,7 @@ ignore: - ModelBiasJobDefinition - ModelExplainabilityJobDefinition - ModelPackage - - ModelPackageGroup + # ModelPackageGroup - ModelQualityJobDefinition - MonitoringSchedule - NotebookInstanceLifecycleConfig @@ -46,7 +58,7 @@ ignore: - PresignedNotebookInstanceUrl - ProcessingJob - Project - - TrainingJob + # TrainingJob - TransformJob - TrialComponent - Trial diff --git a/templates/crossplane/pkg/conversions.go.tpl b/templates/crossplane/pkg/conversions.go.tpl index 72e7743e..818b7168 100644 --- a/templates/crossplane/pkg/conversions.go.tpl +++ b/templates/crossplane/pkg/conversions.go.tpl @@ -53,5 +53,5 @@ func Generate{{ .CRD.Ops.Delete.InputRef.Shape.ShapeName }}(cr *svcapitypes.{{ . // IsNotFound returns whether the given error is of type NotFound or not. func IsNotFound(err error) bool { awsErr, ok := err.(awserr.Error) - return ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }} + return ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }} } \ No newline at end of file diff --git a/templates/pkg/resource/sdk_find_get_attributes.go.tpl b/templates/pkg/resource/sdk_find_get_attributes.go.tpl index ea253c5c..2c07a987 100644 --- a/templates/pkg/resource/sdk_find_get_attributes.go.tpl +++ b/templates/pkg/resource/sdk_find_get_attributes.go.tpl @@ -27,7 +27,7 @@ func (rm *resourceManager) sdkFind( {{- end }} rm.metrics.RecordAPICall("GET_ATTRIBUTES", "{{ .CRD.Ops.GetAttributes.ExportedName }}", respErr) if respErr != nil { - if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{ + if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{ return nil, ackerr.NotFound } return nil, respErr diff --git a/templates/pkg/resource/sdk_find_read_many.go.tpl b/templates/pkg/resource/sdk_find_read_many.go.tpl index cfa78cf5..a31fe502 100644 --- a/templates/pkg/resource/sdk_find_read_many.go.tpl +++ b/templates/pkg/resource/sdk_find_read_many.go.tpl @@ -20,7 +20,7 @@ func (rm *resourceManager) sdkFind( {{- end }} rm.metrics.RecordAPICall("READ_MANY", "{{ .CRD.Ops.ReadMany.ExportedName }}", respErr) if respErr != nil { - if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{ + if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{ return nil, ackerr.NotFound } return nil, respErr diff --git a/templates/pkg/resource/sdk_find_read_one.go.tpl b/templates/pkg/resource/sdk_find_read_one.go.tpl index 16c1b0f3..acd363bc 100644 --- a/templates/pkg/resource/sdk_find_read_one.go.tpl +++ b/templates/pkg/resource/sdk_find_read_one.go.tpl @@ -27,7 +27,7 @@ func (rm *resourceManager) sdkFind( {{- end }} rm.metrics.RecordAPICall("READ_ONE", "{{ .CRD.Ops.ReadOne.ExportedName }}", respErr) if respErr != nil { - if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{ + if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{ return nil, ackerr.NotFound } return nil, respErr diff --git a/templates/pkg/resource/sdk_update_set_attributes.go.tpl b/templates/pkg/resource/sdk_update_set_attributes.go.tpl index ccb8a21b..8fcaf39f 100644 --- a/templates/pkg/resource/sdk_update_set_attributes.go.tpl +++ b/templates/pkg/resource/sdk_update_set_attributes.go.tpl @@ -24,7 +24,7 @@ func (rm *resourceManager) sdkUpdate( _, respErr := rm.sdkapi.{{ .CRD.Ops.SetAttributes.ExportedName }}WithContext(ctx, input) rm.metrics.RecordAPICall("SET_ATTRIBUTES", "{{ .CRD.Ops.SetAttributes.ExportedName }}", respErr) if respErr != nil { - if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessagePrefixCheck .CRD 404 }}{ + if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{ // Technically, this means someone deleted the backend resource in // between the time we got a result back from sdkFind() and here... return nil, ackerr.NotFound