amazon-archives / aws-service-operator

AWS Service Operator allows you to create AWS resources using kubectl.
Apache License 2.0
733 stars 103 forks source link

Consider using Golang templates to augment Cloudformation #128

Open joerocklin opened 5 years ago

joerocklin commented 5 years ago

While researching a solution for #126 I discovered a few restrictions with CloudFormation which make me wonder if it would be better to use golang templates to perform variable substitution in addition to (or perhaps in place of) CloudFormation parameters.

Specifically, I'm hitting an issue where the DeletionPolicy MUST be a string and cannot be a parameter which includes a string (see this AWS forum post about the issue which was created in May 2014). The workarounds for this may make sense for someone working with CloudFormation directly, though that debate is outside the scope of this tool.

In the case ot the Service Operator, where the tempalte is being read in from a file before being sent to AWS, there is an opportunity to perform variable substition or do anything other processing. In the case of the DeletionPolicy, performing a quick template substitution with the data already provided from the k8s manifest would yield a clean solution presented to CloudFormation - no workarounds needed.

I'm not incredibly familiar with CloudFormation, but in this case it seems like more of a means to an end of abstracting the lifecycle of resources in AWS away from users of a kubernetes cluster. Because of the DeletionPolicy scenario (and the limit of 60 parameters - not something which would easily be hit, but I could foresee the possibility with more complex abstractions), I would like to propse the consideration of adding a phase to the processing of the CloudFormation template file to perform golang template substitution.

christopherhein commented 5 years ago

Hey @joerocklin, Very nice work on the #129 this is something that I originally started with, it's also how we started the awslabs/aws-servicebroker and the downfall actually ends up being that we hit POST request body limits with the CFN API on some of the larger templates. Thus the usage of the files directly from S3. The other thing using the parameters gives you is the ability to diff the resource to the stack in CFN and only update when something has actually changed.

In trying to fix your solution I think I have an already implemented mechanism; if you check out - https://github.com/awslabs/aws-service-operator/blob/master/cloudformation/s3bucket.yaml#L32-L38 you'll see where we set up the params then if you notice where we reference them in the model file you'll see it thinks the attribute it a standard yaml bool https://github.com/awslabs/aws-service-operator/blob/master/models/s3bucket.yaml#L28-L34 being that CFN expects everything to be a string I just mutate all types through the helpers.Stringify() func - https://github.com/awslabs/aws-service-operator/blob/master/pkg/helpers/helpers.go#L31-L41.

In theory you should just have to add the new param in the cloudformation/s3bucket.yaml and change the resource to use that param, then add the new property in the model file and re-code gen the models and it should work.

joerocklin commented 5 years ago

I tried an implementation of setting the DeletionPolicy before embarking on this effort but failed CFN validation every time.

This resulted in the following error from the CFN validation (this from running the file through the validator in the AWS console):

10/23/2018, 6:27:56 AM - Template contains errors.: Template format error: Every DeletionPolicy member must be a string.

The discussion in the AWS forum post linked above shows that the DeletionPolicy doesn't accept a reference to a string. I even tried to put some if statements around things, changing the parameter to a boolean and trying to put in bare keywords based on the state of the boolean - this too failed validation with the same error about needing to be a string. I don't think there are viable workarounds for this issue using CFN alone.

I see the size limitations of the TemplateBody method I've used. Would an acceptable workaround be to indicate in the CFN resource definition whether or not golang substitution was requried. Defaulting to false should allow all existing scenarios to work as described but also allow for individual override.

christopherhein commented 5 years ago

Not sure if this is the issue, but in https://github.com/joerocklin/aws-service-operator/blob/delete-v2/cloudformation/s3bucket.yaml#L154 needs to be tabbed in one more time to apply to the S3Bucket resource in the CFN.

joerocklin commented 5 years ago

Just to make sure I didn't have strange artifacts of some other efforts, I went back and started the implementation again. I've tried both the configurations in this gist. The only differences are the indentation on the DeletionPoloicy line:

I'm happy to try other ideas.

joerocklin commented 5 years ago

I realized I missed some context with the above question, and this discussion should really be taking place over on #126.