aws-quickstart / cdk-eks-blueprints

AWS Quick Start Team
Apache License 2.0
446 stars 197 forks source link

NGINX Add-on: Wrong link in the documentation #917

Closed ilunie closed 2 weeks ago

ilunie commented 6 months ago

Describe the documentation issue

Is it just a typo, or is it assumed that it uses Kubernetes Nginx?

On this link https://aws-quickstart.github.io/cdk-eks-blueprints/addons/nginx/, it is described that it uses Kubernetes Nginx. However, in the source code, the main nginx.org packages are employed. although they represent the same nginx, they are implemented differently.

Refer to the source code at https://github.com/aws-quickstart/cdk-eks-blueprints/blob/main/lib/addons/nginx/index.ts#L65. On the other hand, is there any reason why it chooses nginx.org over kubernetes nginx?

Links

https://aws-quickstart.github.io/cdk-eks-blueprints/addons/nginx/

ArneOttenVW commented 6 months ago

This threw me off big. I kept getting errors regarding ingress rule merging which only the kubernetes-nginx seems to be capable of? At least according to: https://stackoverflow.com/a/59664999

shapirov103 commented 6 months ago

@ArneOttenVW you can merge ingress with the nginx ingress by using "master/minion" ingresses. Here is an example of a main ingress: https://github.com/shapirov103/appmod-blueprints/blob/main/deployment/addons/nginx-main-ingress/main-ingress.yaml

And here is minion with path rewrite (it is embedded into cue lang for Kubevela): https://github.com/shapirov103/appmod-blueprints/blob/main/platform/traits/path-based-ingress.cue#L43

With that said, nothing precludes us from adding another addon for Kubernetes ingress. It will take time to achieve the same feature parity though.

elamaran11 commented 6 months ago

We have an Issue for Kubernetes Nginx Ingress and also fixing the comment issue - https://github.com/aws-quickstart/cdk-eks-blueprints/issues/926. Please upvote if you are interested @ilunie

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

elamaran11 commented 2 weeks ago

This is complete.