aws-containers / retail-store-sample-app

Sample application for demonstrating container platforms and related technology
MIT No Attribution
229 stars 276 forks source link

fix: added "aws-load-balancer-scheme" annotation to UI Service #579

Closed albycrescini closed 2 months ago

albycrescini commented 2 months ago

Issue #, if available: During the deployment of the Helm chart, I observed that the UI service was creating an Internal Load Balancer. Although a public URL was provided, the service was inaccessible from the internet.

Description of changes: To address this, I have added the annotation "aws-load-balancer-scheme" to "internet-facing" to the deploy.yaml file in the Kubernetes configuration.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

niallthomson commented 2 months ago

Hi @albycrescini thanks for the pull request. I think the idea of of the change is reasonable but theres a couple of things.

First that YAML file was designed to be deployed in to any Kubernetes cluster, like minikube, and not just EKS. So thats the reason all the annotations are missing.

Second is that this file is auto-generated and the exact change you made would get wiped out when the next release is published.

There are some structural changes I'm thinking about that could make this more doable but I won't be able to merge this PR as-is.