Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

Consider using Golang templates to augment Cloudformation #128

Open
joerocklin opened this issue Oct 22, 2018 · 5 comments
Open

Consider using Golang templates to augment Cloudformation #128

joerocklin opened this issue Oct 22, 2018 · 5 comments
Labels
operator/s3bucket S3 Bucket operator package

Comments

@joerocklin
Copy link
Contributor

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
Copy link
Contributor

christopherhein commented Oct 23, 2018

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.

@christopherhein christopherhein added the operator/s3bucket S3 Bucket operator package label Oct 23, 2018
@joerocklin
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
operator/s3bucket S3 Bucket operator package
Projects
None yet
Development

No branches or pull requests

2 participants