mojaloop / project

Repo to track product development issues for the Mojaloop project.
Other
22 stars 15 forks source link

Helm ingress rules at centralledger/chart-service hardcode ingress values #870

Open lewisdaly opened 5 years ago

lewisdaly commented 5 years ago

Summary: In the helm charts at centralledger/chart-service/templates/ingress.yaml, the ingress hosts are hardcoded to refer to .Values.ingress.hosts.api:

...
spec:
  rules:
    - host: {{ .Values.ingress.hosts.api }}
      http:
        paths:
          - path: {{ .Values.ingress.externalPath.api }}
            backend:
              serviceName: {{ $serviceName }}
              servicePort: {{ .Values.containers.api.service.ports.api.externalPort }}
...

This makes it impossible to pass in a values file that specifies 2 or more hosts, for example:

central:
  centralledger:
    centralledger-service:
      ingress:
        hosts:
          api_local: central-ledger.local
          admin_local: central-ledger.local
          api: central-ledger.custom.domain
          admin: central-ledger.custom.domain

Which makes it difficult to configure the central-ledger on a custom domain while retaining the ability to reach the central-ledger using the central-ledger.local hostname.

Severity: Low

Priority: Low

Expected Behavior The ingress hosts should be applied dynamically, such as in ml-api-adapter/chart-service/templates/ingress.yaml:

...
spec:
  rules:
    {{- range $host := .Values.ingress.hosts }}
    - host: {{ $host }}
      http:
        paths:
          - path: {{ $servicePath }}
            backend:
              serviceName: {{ $serviceName }}
              servicePort: {{ $servicePort }}
...

This would allow us to specify multiple hosts to create ingress rules for.

Steps to Reproduce

  1. Deploy the default helm charts
  2. Update the ingress values like so with the above snippet: helm upgrade -f ./ingress.values.yml --repo http://mojaloop.io/helm/repo dev mojaloop
  3. Observe the ingress rules override the central-ledger.local entry instead of adding a new rule:

(this snippet is taken from the dashboard)

...
"rules": [
      {
        "host": "central-ledger.custom.domain",
        "http": {
          "paths": [
            {
              "path": "/",
              "backend": {
                "serviceName": "dev-centralledger-service",
                "servicePort": 80
              }
            }
          ]
        }
      }
    ]
...

Specifications

Notes:

elnyry-sam-k commented 3 years ago

@lewisdaly , @mdebarros thoughts on this?

lewisdaly commented 3 years ago

I'd say it's still a valid issue, but really, it's much easier for users to bring their own ingress than mess around with our 7,000 line long values.yml file