support cloudfront for gateway and static unit #27
Conversation
| // Create an S3 bucket | ||
|
|
||
| const bucketArgs: aws.s3.BucketArgs = {} | ||
|
|
||
| if (indexDocument != '' && !params.cloudFrontEnabled) { |
There was a problem hiding this comment.
does this !params.cloudFrontEnabled still need to exist? The cloudfrontenabled code was moved out but i'm assuming the following code still shouldn't be run if it is enabled?
There was a problem hiding this comment.
yeah youre right, we should still be checking if its enabled here. will need to update
| for _, res := range result.Resources() { | ||
| key := res.Key() | ||
| switch res.(type) { | ||
| case *core.Gateway: |
There was a problem hiding this comment.
nit: i think aside from the a.Config.GetExposed(key.Name) and a.Config.GetStaticUnit(key.Name) the rest of the logic is the same so the switch could just set that and the rest of the logic moved out and de-duped
|
|
||
| for name, keys := range cloudfrontMap { | ||
| cf := resources.CreateCloudfrontDistribution(keys) | ||
| cf.Id = a.Config.AppName + "-" + name |
There was a problem hiding this comment.
it seems like the name value is the same across the app. Is there only a single distribution per app deployment? If i have 1 static unit and 1 gateway, they would be the same cf.Id, is that intentional?
There was a problem hiding this comment.
the name is determined by config. The user has to provide the Id for the CDN. We can look at autogenerating it but my issue with that was that they could name the gateway and static unit the same thing and then we would generate the same name.
I opted for it to be explicit, but we can opt out of that if we think thats best
| }, | ||
| }, | ||
| { | ||
| name: "static unit and gw test", |
There was a problem hiding this comment.
oh, given there is a test for it, i assume the answer to my previous comment is probably yes intentional
There was a problem hiding this comment.
yeah see comment above, lets have a chat about this because this is purely how we want user experience to be and thats the main part i wanted some feedback on
|
no blocking comments. |
|
• Does any part of it require special attention?
• Does it relate to or fix any issue? closes #349
adds cdnId to both gateway and static unit config. This allows us to have logic in the compiler to understand when and how many cloudfront distributions we need with respective origins.
Then the remainder of the logic is handled in deploylib for now until we have a better dependency tree for our aws resources.
We conditionally add the files for static website and cloudfront since we know if we are creating any and just pass in cloudlibs object to avoid more stuff being stuck in compiled
Standard checks