Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: do not pass alias from backend to env controller (#5552)
Addresses: #5479. For both LBWS and Backend, we've been passing whatever that are specified in `http.alias` to env stacks's `Parameters.Aliases`. This is problematic and the cause of #5479. The issue is that `Aliases` is really "PublicAliases", i.e. the aliases that are intended for the public ALB or public CDN. However, we are passing the aliases intended for internal ALB (not public) to `Aliases` nevertheless. #### "`Aliases` is really 'PublicAliases'"? To support the claim that "`Aliases` is really 'PublicAliases'" - every usages of `Aliases` parameter in the environment stack indicate that it should contain only public aliases: 1. https://github.com/aws/copilot-cli/blob/8de7d2b1e4ca83399d0652b43f13c82849d1d0b6/internal/pkg/template/templates/environment/partials/cdn-resources.yml#L46 This is used for the public CDN, so only the public aliases should be passed here. 2. https://github.com/aws/copilot-cli/blob/8de7d2b1e4ca83399d0652b43f13c82849d1d0b6/internal/pkg/template/templates/environment/partials/custom-resources.yml#L30 This is to pass the alias to the Copilot-managed HTTPS certificate. Since internal ALB (for backend) alias only works with imported certificate for now, the HTTPS certificate does _not_ expect any internal alb alias either. Even if we do support managed certificate with internal alb, it should be __a separate certificate__, not this `HTTPSCert`. 3. https://github.com/aws/copilot-cli/blob/8de7d2b1e4ca83399d0652b43f13c82849d1d0b6/internal/pkg/template/templates/environment/partials/custom-resources.yml#L44 The `CustomDomainAction` clearly does not intend to take care of an internal ALB, judging from `PublicAccessDNS: !GetAtt PublicLoadBalancer.DNSName`. Again, similar to 2, even if we do want to support upserting a-records for internal ALB aliases, we need to refactor this custom action, as well as env controller, such that it is possible for the env stack to differentiate an alias for a public ALB from an alias for an internal ALB. To recap, _even if we want to support managed aliases (i.e. aliases for which we manage the certificate and the A-records)_, we would need to pass pass the `http.alias` of a backend service to the env stack, in a way that __it is obvious that those aliases are intended for the internal ALB__. For example, I could imagine that we'd want to pass it to `Parameters.InternalALBAliases`, a new not-existing-yet parameter, instead of to `Parameters.Aliases`. #### My point 😂 All these explanations are to say that - the reason why we shouldn't pass `EnvControllerAction.Properties.Aliases` from the backend service is __not because__ the internal ALB only works with imported certs, _but_ because the internal ALB aliases do not belong in `Parameters.Aliases` in any cases. A related observation: LBWS is passing `EnvControllerAction.Properties.Aliases` (hence to `Parameters.Aliases`) even if the environment has imported cert. Is this a break of code pattern? No - even if we still want to pass the Backend's `http.alias` to the environment, like I said, it shouldn't be passed over `Parameters.Aliases`, but `Parameters.InternalALBAliases`. The notes are left for fellow maintainers if they stumble upon the same confusion in the future! By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
- Loading branch information