Merge multiple ingresses with the same name from different includes

[csteeg]

What should we add or change to make your life better?

If we include multiple tye files from 1 main tye file, we cannot easily add ingresses on the same port atm. Take this as an example: main tye.yaml:

name: myenv
services:
- name: myauthservice
  include: .\myauthserice\tye.yaml
- name: blazorappweb
  include: .\blazorappweb\tye.yaml

.\myauthserice\tye.yaml:

name: myenv

ingress:
  - name: ingress
    bindings:
      - port: 80
        ip: "*"
        protocol: http
    rules:
      - host: login.mydomain.local
        service: Authentication

services:
- name: Authentication
  project: source\Authentication\Authentication.csproj
  env_file:
    - ../.env

\blazorappweb\tye.yaml:

name: myenv

ingress:
  - name: ingress
    bindings:
      - port: 80
        ip: "*"
        protocol: http
    rules:
      - host: app.mydomain.local
        service: BlazorApp

services:
- name: BlazorApp
  project: source\BlazorApp\BlazorApp.csproj
  env_file:
    - ../.env

Now if you try to startup the main tye file, you'll get an exception that 'ingress' is already added to the services dictionary in ApplicationBuilderExtensions.ToHostingApplication If you give them a unique name, tye tries to startup 2 webhosts with the same binding, resulting in a port is already in use exception. Specifying 1 ingress in the main file is also not possible, since the validator checks if the ingress points to an existing service in the same file, but the actual service where it should point to isn't in the same file But having the ingress defined in the own tye files seems better to me anyway, so the can also run standalone.

Why is this important to you?

To have tye list on 1 port and and ip and serve all loaded services from that port based on domain-name

[davidfowl]

I see, you want to do a config merge. I'm not sure this should just work, maybe this needs to be declared "partial" in order to allow the merge. We need to be able to distinguish the error case (oops I didn't mean to overwrite) from the merge case.

[csteeg]

@davidfowl what happens without this PR, is that it throws an exception for the second ingress it finds:

An unhandled exception has occurred, how unseemly:
System.ArgumentException: An item with the same key has already been added. Key: ingress

and if you give them a unique name:

An unhandled exception has occurred, how unseemly:
System.IO.IOException: Failed to bind to address http://[::]:80: address already in use.

Your suggestion is to keep throwing these exception (or another one) unless you explicitly say they can be merged? So perhaps the main tye should have the ingress with bindings but without any rule and a 'allowIncludeRules' like so?

name: myenv
ingress:
  - name: ingress
    allowIncludeRules: true
    bindings:
      - port: 80
        ip: "*"
        protocol: http

services:
- name: myauthservice
  include: .\myauthserice\tye.yaml
- name: blazorappweb
  include: .\blazorappweb\tye.yaml

I don't think much harm could be done by adding rules to the same ingress though. Or are you more concerned about overwring the bindings?

We could also throw an exception if the name matches but the bindings don't -> InvalidConfigurationException: "A conflict between the ingresses in myauthservice and blazorappweb has been found. The name of the ingresses are the same, but the bindings are not"

• one for overlapping rules' hostnames and path: InvalidConfigurationException: "blazorappweb has an ingress rule for app.mydomain.local/myapp, but the same rule was already specified by 'otherwebapp'"

So we have a option 1: add extra property to ingress to allow 'sub-rules' option 2: only throw an exception if there are conflicts after a merge (where you'd get an exception in the current version always)

I prefer option 2, only throw an exception with conflicting rules

[csteeg]

I added exceptions for the conflict situations and added unittests for non-conflict and conflict situation