Consider what to do about environment variable clashes

[ghost]

When running a Spring Boot application on a server that has any environment variable with a prefix of "SERVER_", the ServerProperties class will attempt to use it as a property and subsequently break due to the ignoreUnknownFields = false annotation...

Is it possible to configure the ServerProperties class to have ignoreUnkownFields = true instead (as this seems like the situation that it was designed for)?

Otherwise is there another way around the issue, bearing in mind that I'm currently using the environment variable SPRING_PROFILES_ACTIVE to control which type of environment the app should be running as (dev, stage, prod, etc).

Caused by: org.springframework.beans.NotWritablePropertyException: Invalid property 'PORT_BREAK' of bean class [org.springframework.boot.autoconfigure.web.ServerProperties]: Bean property 'PORT_BREAK' is not writable or has an invalid setter method. Does the parameter type of the setter match the return type of the getter?

[ghost]

https://github.com/spring-projects/spring-boot/blob/master/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java

is the offending class.

[philwebb]

@dsyer @wilkinsona Seems like a sensible change for 1.2.x. Any objections?

[snicoll]

Is that really fixing the underlying issue? I mean, we could have the same kind of scenario with virtually anything, right? Of course, SERVER_ is likely to be more used than SPRING_ for instance but the problem remains the same.

[dsyer]

Seems like a reasonable step for 1.2.6 though. I was never very comfortable that we didn't use ignoreUnknownFields=false more (it's a valuable feature). So for 1.3 maybe we could do something more thorough?

[wilkinsona]

I don't like it. The proposed change would mean that everyone loses the assurance that their configuration keys match up just to address a conflict in one person's environment. Perhaps we have no choice, though – server. has always been a very generic prefix. IMO, this is the best argument I've seen for moving everything beneath a Boot-specific prefix.

[snicoll]

cough cough

[philwebb]

Hmm, we almost want ignoreUnknownFields to only apply to application.properties values. I still think we should make this change for 1.2.6 and come up with something better for 1.3.0.

[snicoll]

Hmm, we almost want ignoreUnknownFields to only apply to application.properties values.

Not really, you get auto-completion in the IDE now so the only places really when you need some kind of safety net are System property, OS env variable or command line switch. And this is even more true when you start to deviate from the canonical format.

I'd actually argue for the opposite. Of course, that doesn't help here :(

[dsyer]

For me one of the main benefits of having an ignoreUnkownFields flag is for people without an IDE (e.g. operators or non-Java users writing config files or setting env vars). They need the safety net, and it's already heavily devalued because we don't use it enough (for good reasons). IMO we need a better solution than just switching it off. I think I just agreed with @snicoll.

[sbrannen]

IMO, this is the best argument I've seen for moving everything beneath a Boot-specific prefix.

@wilkinsona, I couldn't agree more!

[sbrannen]

Hi,

I've put a considerable amount of thought into this topic, and since it recently affected me personally (see #3775) I feel it's time to share my thoughts and concerns.

Summary:

Spring Boot's opinionated view of the world of Spring is great! But... that opinionated view simply must not extend beyond the boundaries of a Spring application since the consequence is a guaranteed recipe for disaster.

Rationale:

In retrospect, it was an unfortunate design decision to bind Spring Boot configuration properties to non-prefixed variables in the environment.

Spring Boot knows how a Spring application should be best configured; but Spring Boot does not know how the enclosing environment (server, developer workstation, container, etc.) should be best configured.

In fact, Spring Boot should not make any assumptions about or place any restrictions on the environment in which a Spring Boot application is deployed. Any attempt to do so will make Spring Boot deployments fragile at best, likely hindering adoption in certain companies.

As a concrete example, let's assume that we are developing a Spring Boot based web application that will start an embedded Tomcat server on deployment. We would like for this fat JAR to be able to be deployed in various environments, even if we are not in control of the deployment environments. We therefore cannot place any restrictions on the presence of environment variables already present in such a deployment environment. The system administrator or operations team may deem it necessary to define environment variables such as SERVER_HOME, SERVER_PORT, etc. These variables may be required by other applications running in the same environment which have absolutely nothing to do with Spring, Spring Boot, or potentially even Java. Furthermore, and perhaps more importantly, there may be components or libraries deployed within the Spring Boot app that rely on the presence of those environment variables. In other words, the removal (or renaming) of such environment variables just to allow Spring Boot to start is not an option.

Essentially, if a sysadmin needs to define SERVER_HOME, SERVER_PORT, etc. on a system under their control, that is their prerogative, and Spring Boot simply must not forbid that.

We can learn a lesson from the core Spring Framework here: when we introduced support for environment profiles several years ago, we decided to introduce a 'spring. prefix for Java system properties -- for example, spring.profiles.default instead of merely profiles.default (or SPRING_PROFILES_DEFAULT vs. PROFILES_DEFAULT as environment variable names). The reason is that a variable name like PROFILES_DEFAULT is just too generic: Spring would never presume that it is the only application in the world that would use a PROFILES_DEFAULT environment variable.

The same arguments hold true for Spring Boot: Spring Boot cannot presume that it is the only application in the world that would define an environment variable named SERVER_PORT or SERVER_*.

The only viable solution to make Spring Boot a good citizen within the world of IT is to introduce a prefix specific to Spring Boot (proposal: "spring.boot."): for example, SERVER_PORT should be renamed to SPRING_BOOT_SERVER_PORT

That's how I see it, and I hope we can come up with a workable solution for this topic in time for Spring 1.3 GA!

Regards,

Sam

[spencergibb]

Would #3450 cover some of the concerns?

[sbrannen]

@spencergibb, #3450 would be one option, but it's limited in its applicability since it requires that a developer be aware of the problem to begin with. My goal is that all Spring Boot applications (including those created by developers new to Spring Boot with wizards like http://start.spring.io/) should work without modification regardless of what environment variables are present but unrelated to Spring Boot.

[wilkinsona]

We talked about this a bit on today's Boot team call. Neither prefixing everything nor #3450 are good solutions for 1.3:

• Prefixing everything would be a big breaking change. We may consider it for 2.0.
• Add prefix support for environment variables #3450 would help people writing new apps, but doesn't solve the problem for existing apps that had, until now, been running happily in an environment with SERVER_HOME (for example) configured.

Our current plan is to investigate offering two different levels of strictness when binding configuration properties. The current behaviour (in 1.3.0.M4) blows up if a property from any source cannot be bound and ignoreUnknownProperties=false has been set. We'd like to introduce another less strict level that would only blow up if the source is within the application's control: application.properties, command line arguments, etc, but would not blow up for OS-wide sources such as environment variables.

[sbrannen]

Prefixing everything would be a big breaking change. We may consider it for 2.0.

Agreed! I vote for that change in 2.0 with a viable migration path in the interim.

We'd like to introduce another less strict level that would only blow up if the source is within the application's control

That sounds very sensible.

If you don't add a "less strict level" that is used by default in the 1.3 timeline, I would recommend a command-line option for disabling failure for unsupported properties pulled in via environment variables plus the proposed diagnostic improvements in #3778. That combination would at least give developers a clue as to what went wrong and a means to fix it (without having to modify either their app or their existing environment).

Cheers,

Sam

[prasenjit-net]

It seems good to have a prefix for only system environment source, not for the other sources like application.properties. system env is the place where some conflict can happen with other application also. even there can be two boot application running in a machine will require different value to be set in ENV but any other source can be installation specific.

[philwebb]

Moving this one to 1.3.x (although I don't know that we'll get to it) and opening #3903 to track a simpler fix for 1.2.x.

[philwebb]

See #3903 (comment) for an another example of the problem.

[mbhave]

This looks like a duplicate of #3450. Feel free to reopen if that is not the case.