Variable name conflict in constants.conf / Problem with TLS verification, CN and Environment variable

[ekeih]

Hello,

yesterday I updated our stage environment from 2.8.4 to 2.9.1 and it went horribly wrong. After the update icingaweb2 stopped to display new results/hosts/checks.
In the logs of all masters and satellites I got the following warnings every few minutes:

[2018-07-31 09:43:34 +0200] warning/ApiListener: Unexpected certificate common name while connecting to endpoint 'stage.master01:stage': got 'stage.master01'
Context:
        (0) Handling new API client connection

The funny thing is that it says warning but it really means this is a fatal error and your cluster is unable to connect.

After a lot of debugging I found this commit: 9c1e00e and the related PR: #6305

So you may ask: Why does it break our cluster? We already used a Environment=stage variable in our constants.conf. Of course for another purpose. Well, when you know about the PR that introduced the new variable it is pretty obvious that our variable would break the cluster - but I did not know 😉

I have some thoughts about this:

• The warning about the unexpected CN should be an error!
• What is the actual purpose of this variable? I read the PR and the description in the docs but it does not really explain why you added it and what users could use it for. Maybe you could extend the docs about this. (@gunnarbeutner)
• Could we add a way to distinguish between variables with a predefiend meaning and new variables defined by the user?
• In our case we added a prefix to our variable companyNameEnvironment.
• We could add a note to the docs with the advise to use a prefix for custom variables to avoid conflicts with new predefined variables in the future.
• (Calling values in constants.conf variables is confusing.)

I am looking forward to hear other opinions about this. How can we avoid variable name conflicts in the future? What do you think?

Best regards
Max

Your Environment

• Version used (icinga2 --version): r2.9.1-1
• Operating System and version: Ubuntu 16.04.5 LTS

[nbuchwitz]

Any chance that you test the snapshot packages in your testing environment? There was a revert of most changes regarding the Environment vars: ddc5b95

[dnsmichi]

This variable will definitely come back, no need to test that.

It is unfortunate that the global constant namespace is shared with users and system variables. I don't have time atm to explain the changes in 2.9 a bit better, I will come back later with additional thoughts.

[ekeih]

Thank you both for your feedback. The (removed) documentation in ddc5b95 was really helpful to understand why you added this feature 🙂

It is unfortunate that the global constant namespace is shared with users and system variables. I don't have time atm to explain the changes in 2.9 a bit better, I will come back later with additional thoughts.

We fixed it for our setup by changing the name of our variable. So it is not urgent for us ;)

[lazyfrosch]

@dnsmichi is correct, this feature will come back, in a slightly minimalistic way.

Environment will stay a constant that influences SNI for TLS connections.

You should always use a prefix for constants, to avoid a possible clash with our internals.

I guess I'll add a note within the docs.

[ekeih]

Could we add a way to distinguish between variables with a predefiend meaning and new variables defined by the user?

#6509 looks related 🤔

I guess I'll add a note within the docs.

Thank you! :)

[gunnarbeutner]

Well, yes, unfortunately that's only part of the solution. Icinga already has partial support for namespaces and that PR is yet another push to get "proper" namespacing support at some point.

[dnsmichi]

Two tasks here:

• Add Environment to the upgrading docs, being reserved for future usage
• Fix the location where the error message sources from. This compares <FQDN>:<Environment> with ´<FQDN>. This only happens if Environment is set to something else than production in 2.9.x.

[dnsmichi]

Will be fixed as part of #6512 in my review cycle.