Implement AWS SES Template #755

ikarldasan · 2021-07-01T17:24:20Z

Description of your changes

Adds Support for AWS SES Template

"Fixes a part of #414 "

I have:

Read and followed Crossplane's [contribution process].
Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Manually tested the creation, updation and deletion of Template

Signed-off-by: Karl Dasan <[email protected]>

chlunde

Nice work! I'm not a maintainer and I haven't used code gen, but I think I found two small issues you should look at, hope this helps.

chlunde · 2021-07-02T18:16:49Z

examples/ses/template.yaml

+    name: example
+spec:
+    forProvider:
+    region: us-east-2


indent this section under forProvider, and check that it works

Indented the section under forProvider .
Checked for Create, Update and Delete operations - Works successfully.

chlunde · 2021-07-02T18:45:50Z

pkg/controller/ses/template/setup.go

+
+func preObserve(_ context.Context, cr *svcapitypes.Template, obj *svcsdk.GetTemplateInput) error {
+	obj.TemplateName = aws.String(meta.GetExternalName(cr))
+	cr.Spec.ForProvider.Template.TemplateName = obj.TemplateName


I think you should add this field to ignore in code gen. Look at lambda for example, the field FunctionName is ignored, so it's only part of external-name and not from the cr.
https://github.com/crossplane/provider-aws/blob/36e425acdcb52a670f78ab5e37b8f81864d3f393/pkg/controller/lambda/function/setup.go#L87-L90

https://github.com/crossplane/provider-aws/blob/36e425acdcb52a670f78ab5e37b8f81864d3f393/apis/lambda/v1alpha1/generator-config.yaml#L1-L3

The CreateTemplateInput struct contains a *Template which contains the TemplateName.

Do you suggest adding TemplateName to ignore field_paths like this?

ignore: field_paths: - CreateTemplateInput.Template.TemplateName

I tried adding the same. The resultant code generated after re running make generate does not have the Template Parameters in the ForProvider i.e the CRD does not accept the Template_SDK struct that contains HTMLPart, SubjectPart, TemplateName, TextPart

I think there is a need to update the code-generator, it does not support three levels like Foo.Bar.Baz.

I made a (draft) PR here: aws-controllers-k8s/code-generator#119

@muvaf points out that this can be achieved already using

ignore: field_paths: - Template.TemplateName

Again I would like to point out that I'm not 100% sure this is the correct approach as I've not used code gen. :)

chlunde · 2021-07-02T18:47:10Z

examples/ses/template.yaml

+        subjectPart: "sample subject part"
+        textPart: "sample text part"
+    providerConfigRef:
+    name: example


nitpick: if all other files have a final newline, it would be nice to add it to this file and generator-config.yaml too.

Added newline to the example.

Signed-off-by: Karl Dasan <[email protected]>

AaronME

Works well to create, delete.

Update works for changes to SubjectPart, TextPart, and HtmlPart.

I've included some thoughts on how to handle an update to TemplateName.

AaronME · 2021-07-23T15:13:00Z

examples/ses/template.yaml

+apiVersion: ses.aws.crossplane.io/v1alpha1
+kind: Template
+metadata:
+    name: example


Tiny nitpick:

Our other examples are using 2-space indents. Can we reformat this to match?

AaronME · 2021-07-23T19:21:30Z

pkg/controller/ses/template/setup.go

+}
+
+func isUpToDate(cr *svcapitypes.Template, obj *svcsdk.GetTemplateOutput) (bool, error) {
+	if !cmp.Equal(cr.Spec.ForProvider.Template.TemplateName, obj.Template.TemplateName) {


Since we cannot update the templateName once the Template is created, we may want to...

Ignore Template.TemplateName in code generation and use PreObserve, PreCreate, PreUpdate, and PreDelete to always set it to cr.Name.

Return an informative error (eg, "Cannot update field TemplateName, ignoring.") and return true

Delete the Template and allow re-create with the new name.

AaronME · 2021-07-23T20:05:49Z

examples/ses/template.yaml

+    forProvider:
+        region: us-east-2
+        template:
+            templateName: example


I tested updating spec.forProvider.template.templateName on an existing Template resource and received the following error:

2021-07-23T15:24:19.702-0400 DEBUG provider-aws Cannot update external resource {"controller": "managed/template.ses.aws.crossplane.io", "request": "/example", "uid": "b15f8ce9-c765-4543-bf1c-435caaaa8bc6", "version": "50082", "external-name": "example"} 2021-07-23T15:24:19.702-0400 DEBUG controller-runtime.manager.events Warning {"object": {"kind":"Template","name":"example","uid":"b15f8ce9-c765-4543-bf1c-435caaaa8bc6","apiVersion":"ses.aws.crossplane.io/v1alpha1","resourceVersion":"50082"}, "reason": "CannotUpdateExternalResource", "message": "cannot update Template in AWS: TemplateDoesNotExist: Template example2 does not exist."}

AaronME · 2021-10-01T06:30:32Z

@ikarldasan are you still working on this PR?

haarchri · 2022-06-26T12:40:46Z

@ikarldasan any chance to see this finalized ?

MisterMX · 2023-07-28T14:14:08Z

Closing as this was implemented in #791. Feel free to reopen if that is not the case.

…ib#755)

ikarldasan added 3 commits July 2, 2021 12:19

feat(SES): Add support for SES Template

fb90e40

Signed-off-by: Karl Dasan <[email protected]>

feat(SES): added template example

93daee6

Signed-off-by: Karl Dasan <[email protected]>

feat(SES): fix go fmt

2bce61b

Signed-off-by: Karl Dasan <[email protected]>

ikarldasan force-pushed the ses-temp branch from 0011c8f to 2bce61b Compare July 2, 2021 06:50

chlunde requested changes Jul 2, 2021

View reviewed changes

ikarldasan added 2 commits July 4, 2021 12:17

feat(SES): enhance template example

4558e7c

Signed-off-by: Karl Dasan <[email protected]>

feat(SES): enhance preObserve hook to get the external name

95a9a17

Signed-off-by: Karl Dasan <[email protected]>

AaronME suggested changes Jul 23, 2021

View reviewed changes

AaronME self-assigned this Jul 23, 2021

chlunde mentioned this pull request Aug 3, 2021

Support nested Ignore.FieldPaths aws-controllers-k8s/code-generator#119

Closed

ikarldasan mentioned this pull request Aug 6, 2021

Implement Simple Email Service (SES) resources #414

Closed

AaronME added the size/S label Aug 12, 2021

MisterMX closed this Jul 28, 2023

tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024

Handle build environment variables for proxy access (crossplane-contr…

23b2607

…ib#755)

Implement AWS SES Template #755

Implement AWS SES Template #755

Uh oh!

Conversation

ikarldasan commented Jul 1, 2021

Description of your changes

How has this code been tested

Uh oh!

chlunde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikarldasan Jul 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chlunde Aug 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikarldasan Jul 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronME left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronME commented Oct 1, 2021

Uh oh!

haarchri commented Jun 26, 2022

Uh oh!

MisterMX commented Jul 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ikarldasan Jul 4, 2021 •

edited

Loading

chlunde Aug 3, 2021 •

edited

Loading

ikarldasan Jul 4, 2021 •

edited

Loading