harbor-helm icon indicating copy to clipboard operation
harbor-helm copied to clipboard

Ingress - Explicit rules order cause incorrect behaviour with AWS ALB using aws-load-balancer-controller

Open terghalin opened this issue 3 years ago • 4 comments

Current harbor-helm chart (2.2.0 and higher) has a root path rule for harbor-portal on top of others, which makes it a priority rule while using AWS ALB.

    - path: /*
      backend:
        service:
          name: harbor-{fqdn-name}-harbor-portal
          port:
            number: 80
...
    - path: /c/*
      backend:
        service:
          name: harbor-{fqdn-name}-harbor-core
          port:
            number: 80

According to AWS documentation, "Rules are evaluated in priority order, from the lowest value to the highest value".

I think it's better to add specific ALB template or just move harbor-portal rule to the bottom, no other controller types should be affected: https://github.com/goharbor/harbor-helm/blob/master/templates/ingress/ingress.yaml

terghalin avatar May 11 '21 14:05 terghalin

There is a PR introducing the support for AWS ALB https://github.com/goharbor/harbor-helm/pull/914

ywk253100 avatar May 13 '21 10:05 ywk253100

PR #914 addresses path syntax quirks with the AWS Load Balancer Controller in v2.1.3 and earlier. It does not address the path priority issue noted above. Shuffling the harbor-{fqdn-name}-harbor-portal rule to the bottom of the ingress definition is an additional correction that allows the harbor login page to load correctly, at least it does in my environment.

Previously I had noted that I was still having issues after shuffling the Ingress paths order... this just a result of a misconfigured OIDC endpoint URL. Pushing the portal '/' path to the bottom of the rules definitely works for the AWS Load Balancer Controller!

jgregmac avatar May 16 '21 16:05 jgregmac

PR #1006 address this explicitly without adding any features specific to ALBs. The change in this PR is not included in #914 and would be required for #914 to work correctly as well.

PR #1006 allows use of the existing Ingress resources with the v2.x series of the AWS Load Balancer Controller without any other changes.

democracytoday avatar Jul 14 '21 22:07 democracytoday

It's also worth noting that in the meantime you can emulate the functionality of PR #1006 with the following Helm post renderer, here specified as part of a GitOps Toolkit / Flux2 HelmRelease:

        patchesJson6902:
          # Reorder Ingress rules to work with AWS Load Balancer
          # Moves all-matching '/*' rule to lowest priority position
          # Can remove when merged: https://github.com/goharbor/harbor-helm/pull/1006
          - target:
              group: networking.k8s.io
              version: v1
              kind: Ingress
              name: harbor-ingress
            patch:
              - op: replace
                path: /spec/rules/0/http/paths/0/path
                value: /c/
              - op: replace
                path: /spec/rules/0/http/paths/0/backend/service/name
                value: harbor-core
              - op: replace
                path: /spec/rules/0/http/paths/5/path
                value: /
              - op: replace
                path: /spec/rules/0/http/paths/5/backend/service/name
                value: harbor-portal

democracytoday avatar Jul 26 '21 14:07 democracytoday

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.

github-actions[bot] avatar Feb 08 '24 09:02 github-actions[bot]

This issue was closed because it has been stalled for 30 days with no activity. If this issue is still relevant, please re-open a new issue.

github-actions[bot] avatar Mar 11 '24 09:03 github-actions[bot]