add support for generic callback hooks

[jaypipes]

This patch adds support for a generic callback system into the generator
config that allows controller implementors to specify some code that
should be injected at specific named points in a template. I expect that
eventually this generic hook system will be more useful, flexible and
extensible than the hodge-podge of custom callback methods and overrides
currently in the generator config.

The pkg/generate/config.ResourceConfig struct now has a Hooks field
of type map[string]*HookConfig, with the map keys being named hook
points, e.g. "sdk_update_pre_build_request".

There are two ways to inject code at hook points: inline and via a
template path. The inline method uses the HookConfig.Code field which
should contain the Go code that gets injected at a named hook point. The
HookConfig.TemplatePath field is used to refer to a template file at a
specific path. The template file is searched for in any of the
TemplateSet's base template paths.

Here's an example of a generator config snippet that uses the inline
code injection method (HookConfig.Code) to add a piece of custom code
to be executed in the sdk_update.go.tpl right before the code in the
resource manager's sdkUpdate method calls the
newUpdateRequestPayload() function:

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        code: if err := rm.requeueIfNotRunning(latest); err != nil { return nil, err }

Here is the snippet from the templates/pkg/resource/sdk_update.go.tpl
file that shows how we can add these generic named hooks into our
templates at various places:

{{- if $hookCode := .CRD.HookCode sdk_update_pre_build_request }}
{{ $hookCode }}
{{ end -}}

The controller implementor need only implement the little
requeueIfNotRunning function, with the function signature described
in the generator config code block. We no longer need to have function
signatures match for custom callback code since the function signature
for callback code is up to the dev writing the generator.yaml config
file.

Here is an example of the HookConfig.TemplatePath being used to refer
to a template file containing some code that is injected at a named hook
point:

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        template_path: sdk_update_pre_build_request.go.tpl

A controller implementor would simply need to populate a
sdk_update_pre_build_request.go.tpl file with the code to be included
at the hook point.

This patch introduces the following hook points in the ACK controller
resource manager code paths:

• sdk_read_one_pre_build_request
• sdk_read_many_pre_build_request
• sdk_get_attributes_pre_build_request
• sdk_create_pre_build_request
• sdk_update_pre_build_request
• sdk_delete_pre_build_request
• sdk_read_one_post_request
• sdk_read_many_post_request
• sdk_get_attributes_post_request
• sdk_create_post_request
• sdk_update_post_request
• sdk_delete_post_request
• sdk_read_one_pre_set_output
• sdk_read_many_pre_set_output
• sdk_get_attributes_pre_set_output
• sdk_create_pre_set_output
• sdk_update_pre_set_output

The "pre_build_request" hooks are called BEFORE the call to construct
the Input shape that is used in the API operation and therefore BEFORE
any call to validate that Input shape.

The "post_request" hooks are called IMMEDIATELY AFTER the API operation
aws-sdk-go client call. These hooks will have access to a Go variable
named resp that refers to the aws-sdk-go client response and a Go
variable named respErr that refers to any error returned from the
aws-sdk-go client call.

The "pre_set_output" hooks are called BEFORE the code that processes the
Outputshape (the pkg/generate/code.SetOutput function). These hooks will
have access to a Go variable named ko that represents the concrete
Kubernetes CR object that will be returned from the main method
(sdkFind, sdkCreate, etc). This ko variable will have been defined
immediately before the "pre_set_output" hooks as a copy of the resource
that is supplied to the main method, like so:

	// Merge in the information we read from the API call above to the copy of
	// the original Kubernetes object we passed to the function
	ko := r.ko.DeepCopy()

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[vijtrip2]

I like it. 🚀

---

My only concern from first look was what if someone takes this hook too literally and writes bulky code blocks in generator config, that will make generator config less readable.
But since they control the signature of hook function and implementation too, this code block should ideally be a single function call encapsulating all the details. : )

[jaypipes]

I like it. rocket

My only concern from first look was what if someone takes this hook too literally and writes bulky code blocks in generator config, that will make generator config less readable.
But since they control the signature of hook function and implementation too, this code block should ideally be a single function call encapsulating all the details. : )

Yeah, agreed. Plus, putting large blocks of code in YAML is actually annoying and fraught with peril, so it kind of self-selects that out ;)

[a-hilaly]

LGTM!

i'm wondering if we should offer a set of hooks to use, instead of allowing users to pass plain code. something like:

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        requeueIfNotRunning: true

or

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        - requeueIfNotRunning
        - anotherHook

wdyt @vijtrip2 @jaypipes ?

[jaypipes]

LGTM!

i'm wondering if we should offer a set of hooks to use, instead of allowing users to pass plain code. something like:

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        requeueIfNotRunning: true

or

resources:
  Broker:
    hooks:
      sdk_update_pre_build_request:
        - requeueIfNotRunning
        - anotherHook

wdyt @vijtrip2 @jaypipes ?

We can think about that at some later time, yes.

[surajkota]

Don't really like the gocode in yaml file. I can't see how hooks is different from custom_implementation except one doesn't need to match the function signature. Developer still need to implement the custom method

Could you provide more information? Or am I missing something?

I can see a pattern for a status field and value to check for before triggering update or successful reconciling but the name of field and value to check will be different for each service

[jaypipes]

Don't really like the gocode in yaml file. I can't see how hooks is different from custom_implementation except one doesn't need to match the function signature. Developer still need to implement the custom method

I would like to make custom_implementation a last resort. The hooks concept is more flexible and able to be fine-tuned so that we reduce the total amount of custom code over time.

I agree that adding Go code in YAML is icky :)

Would you like this PR better if I added a HookConfig.Template field that could be the path to a template file that contained the Go code to be injected at the hook point? For instance, you could create a template file named sagemaker-controller/templates/my_custom_code.go.tpl and then refer to the code in that template file by doing something like this in the sagemaker generator.yaml file:

resources:
  Endpoint:
    hooks:
      sdk_update_pre_build_request:
        template: sagemaker-controller/templates/my_custom_code.go.tpl

Could you provide more information? Or am I missing something?

I can see a pattern for a status field and value to check for before triggering update or successful reconciling but the name of field and value to check will be different for each service

Right, the reason this needs to be custom code for the immediate future is because each API is different in the way it checks for certain state-transition changes... I would love to get to a point where we can describe these state-transition change strategies in a declarative fashion in the generator.yaml file and have the Go code generated that does the checks, but due to the inconsistencies in the APIs, we are quite far away from that ability at the moment so this is a stop-gap measure.

[jaypipes]

@surajkota please see #28 for a patch that enables the idea of having code in template files instead of specified as HookConfig.Code

[vijtrip2]

LGTM.

[surajkota]

Thanks, Lgtm!
not sure if it was intentional, template changes are missing in sdk_find_read_many, sdk_find_get etc.

Will try using this PR today

general question, if one is using hook.code, how will the indentation be handled, is it only good for one-liner code?

[a-hilaly]

/lgtm 👍

[a-hilaly]

general question, if one is using hook.code, how will the indentation be handled, is it only good for one-liner code?

Most indentation problems will be corrected by gofmt.

It is still possible to provide multi-line code with the correct indentation using something like:

hooks:
  sdk_update_pre_build_request:
  code: |
    if err := rm.requeueIfNotRunning(latest); err != nil {
        return nil, err
    }

[RedbackThomson]

I like this idea, but I couldn't come up with any variables that could be useful at compile-time? Possibly version information?

[jaypipes]

Yeah, I left it as a TODO because I couldn't think of anything off top of my head. We can always add later if we stumble on something that should be passed to the template...

[RedbackThomson]

Github formatting or actually ugly?

[jaypipes]

actually ugly :) will correct.

[jaypipes]

Thanks, Lgtm!
not sure if it was intentional, template changes are missing in sdk_find_read_many, sdk_find_get etc.

@surajkota nice catch. Added hooks for read_many and get_attributes.