PortAssigner replaces app/admin connectors

[sleberknight]

PortAssigner completely replaces the application and admin connectors when using secure dynamic ports. It would be better to allow for explicit configuration in YAML to set whatever properties are needed, but still assign secure dynamic ports and apply only the TLS configuration. For example, an application might want to change properties that come from HttpConnectorFactory such as the idleTimeout or the outputBufferSize while still using dynamically chosen ports with TLS properties supplied by the TlsContextConfiguration given to PortAssigner at construction time.

This is not a problem when assigning non-secure (i.e., HTTP) because we simply find the connectors (application and admin) and update the ports to the randomly chosen ones. So, an application using dynamic ports and HTTP can specify explicit configuration in its config.yml but still have the dynamic ports applied.

This has never been a problem in our services only because when we've used secure dynamic ports, we've not needed to change the default values that come from HttpConnectorFactory, and we aren't using any of the properties that we don't override (e.g., we override many properties using the TlsContextConfiguration provided to the PortAssigner but don't change the default values for other things like crlPath).

So, when using secure dynamic ports, the PortAssigner should allow explicit configuration in config.yml except for the ports and the TLS properties. All other properties should be overridable in config.yml.

There is an @implNote on PortAssigner#assignDynamicPorts that mentions this limitation. If this change is implemented that note should be removed.

[sleberknight]

There is a potential problem with using the existing server configuration defined in YAML configuration. If we want to assign secure dynamic ports, but the server configuration in the YAML is defined with non-secure ports, then simply updating the port numbers would not work, since the resulting connectors would be non-secure HTTP.

One way of dealing with this would be to validate the connector types are secure and throw an exception if not. This is not ideal, but at least it guarantees correct behavior. So, someone could define a server configuration like:

server:
  applicationConnectors:
    - type: https
      keyStorePath: example.keystore
      keyStorePassword: example
      validateCerts: false
  adminConnectors:
    - type: https
      keyStorePath: example.keystore
      keyStorePassword: example
      validateCerts: false

Then, we would simply accept all the static configuration and change only the port numbers.

But if someone used type: http in the above, we would throw an exception.

Another issue is that if the PortSecurity is supplied as SECURE and we don't outright replace the application and admin connectors, then we have the problem of which value to accept for specific properties, for example it probably makes sense to prefer the values from YAML over the values from the TlsContextConfiguration. This would require checks in the code to determine whether the YAML has overriden the default values in HttpsConnectorFactory or not. If so, we would use the override values (from YAML), otherwise we would use the ones from TlsContextConfiguration.

Another way to solve some of the above problems is to say that, if PortSecurity is supplied as SECURE, we continue to replace the connectors, but otherwise we only change the port numbers and do nothing else. This would mean you could have a complete YAML configuration except the port numbers, but with the downside of not automatically using the TlsContextConfiguration values. Or, perhaps if TlsContextConfiguration is supplied, we use values from it to replace existing values. But it is not supplied, we do nothing.

[sleberknight]

Another slight variation on those discussed in my earlier comments is to overlay all non-null properties from the TlsContextConfiguration onto the existing connectors, which would need to have been defined in YAML as https. And if the existing connectors are not https, then we assume the connectors must be replaced.

So, assuming secure ports, when the YAML has an explicit configuration and the connectors are https, we assign dynamic ports and overlay the TLS values.

But if there is no explicit server configuration (as in the majority of our own services), then we replace the connectors.

This needs some additional thinking and more details, but I think we can make it work to allow dynamic ports for both secure and non-secure ports, and overlaying TLS properties for secure ports. And when no explicit server configuration is defined, we can handle that too for both secure and non-secure ports while assigning dynamic ports.

[sleberknight]

One more consideration is that the PortSecurity enum has SECURE and NON_SECURE. It would be confusing if NON_SECURE were specified, but we don't actually enforce that. In which case NON_SECURE really means unspecified. So, we could add an UNSPECIFIED to PortSecurity which would imply that we enforce secure and non-secure when specified, and we don't do anything when the port security is UNSPECIFIED.