no-tls should be an explicitly handled case

[ciroque]

Sauce: #120 (comment)

If someone were to typo the TLS Mode in the config, we may wish to fail startup. Couple of thoughts here:

• If the choice is to fail startup, how to best raise an alert that can be seen? Is it better to start in insecure mode (no-tls) or possibly fail to start and have it not be noticed?
• As an implementation detail (hate to be too prescriptive, but in this case I'm going to be), the TLS Modes should be iota-based values, NOT STRINGS. This will help avoid a whole class of issues.

[ciroque]

NOTE: The documentation states that no-tls is the default mode. Just be sure to update the docs if that changes!

[ciroque]

How about this, make this configurable: failOnInsecure where the Controller will fail on startup when no-tls is chosen. The default would be on, turn it off to succeed with starting in no-tls mode.

Something to consider here: the only time no-tls is highly recommended is in production. Through development, and most likely testing, startup failure due to no-tls mode will be irritating at best.

@4141done thoughts?

[4141done]

So I think having the controller default to no-tls is fine and probably does not need an additional flag (if you're going to set the additional flag, you would want to just set the tls-mode right?). The main thing I think is an issue is defaulting to no-tls if the value of tls-mode is specified but not one of the supported values.

I would not want to have a scenario where tls-mode: ca-ntls makes it through code review and then it's running unsecured but the deployer thinks that it is.

My proposal is:

• We document no-tls as the default
• If tls-mode is specified in the config map and is NOT one of the supported values, we fail as loudly as possible in the kubernetes context.

Cases

I think we'd ideally handle the following cases:

• First time application of the ConfigMap -> Don't apply the config, generate an error Event, log errors
• Applying a new ConfigMap when one is already valid and in use -> Don't apply the new config, old config is maintained, generate an Error event, log errors.

These should be true whether or not the controller is installed.

Possible solutions

Admission Control

I think the best approach based on my reading is to gate the application of a ConfigMap on validation of the values. I'm new to that concept but it looks like Admission Control would be the way or Dynamic Admission Control. Implementing Admission controllers would allow us to handle all the above cases

Error event dispatch

This would likely be a smaller lift, but we'd ideally:

• Log an error
• Dispatch an error event
• Annotate the resource with an error status
• Not apply the configuration change under the hood.