Skip to content

@ConfigurationProperties should have an option to fail fast for unresolvable properties #18816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anno1985 opened this issue Oct 30, 2019 · 13 comments
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Milestone

Comments

@anno1985
Copy link

When using placeholders in application.yaml (property files/externalised configuration) together with the @ConfigurationProperties annotation, there should be an option to have Spring fail fast at startup when a defined property is not found.

Example:

example.key: ${MY_ENV_VAR}
@ConfigurationProperties(prefix="example")
public class AcmeProperties {
  public String key;
  // Getters, setters, constructors omitted... 
}

Precondition:
No environment variable MY_ENV_VAR defined.

Current behaviour:
key above is populated with the verbatim String ${MY_ENV_VAR}

Expected/desired behaviour:
An exception is thrown while the application is starting up.

Potential cause:
Hardcoded flag in https://github.com/spring-projects/spring-boot/blob/v2.2.0.RELEASE/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/PropertySourcesPlaceholdersResolver.java#L51

Further information:
https://stackoverflow.com/q/58622189/2018047

NB:
Validating the String with @Validated after (failed) resolution is not the same as checking if resolution fails. Also, when no fail-fast is active, it might potentially be better to mirror System.getenv("MY_ENV_VAR")'s behaviour, and return null instead of the actual placeholder, ${MY_ENV_VAR}, verbatim.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 30, 2019
@philwebb
Copy link
Member

I believe we intentionally set ignoreUnresolvablePlaceholders to true to keep back compatibility with previous releases. I agree it would be nice to have a fail fast option.

@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 30, 2019
@philwebb philwebb added this to the General Backlog milestone Oct 30, 2019
@philwebb
Copy link
Member

philwebb commented Oct 31, 2019

See also #8693 and #13202

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Oct 31, 2019
@philwebb
Copy link
Member

philwebb commented Nov 1, 2019

@mbhave also tracked down some internal discussion we had about this last time around:

From @wilkinsona

Is it by design that the 2.0 binder doesn’t ignore placeholder resolution failures? Someone on Gitter is complaining and I can’t remember if it’s intentional or not. Here’s a little app that starts fine with 1.5.x but fails with 2.0.x:

package com.example.demo;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.ConfigurableApplicationContext;

import com.example.demo.PlaceholderChangeApplication.ExampleProperties;

@SpringBootApplication
@EnableConfigurationProperties(ExampleProperties.class)
public class PlaceholderChangeApplication {

    public static void main(String[] args) {
        ConfigurableApplicationContext context =
                SpringApplication.run(PlaceholderChangeApplication.class,
                        "--com.example.alpha=Value with ${placeholder}");
        System.out.println(context.getBean(ExampleProperties.class).getAlpha());
    }

    @ConfigurationProperties(prefix="com.example")
    static class ExampleProperties {

        private String alpha;

        public String getAlpha() {
            return alpha;
        }

        public void setAlpha(String alpha) {
            this.alpha = alpha;
        }

    }

}

It works in 1.5.x because of this change that was in the early days of Spring Boot: 8922a6b

@philwebb
Copy link
Member

philwebb commented Nov 1, 2019

I think we intentionally rolled this back in 2.0 because the major upgrade was quite onerous already and we didn't want to add more friction.

@camhashemi
Copy link

we also desire this feature. it's easier to tell that a server won't start up if the process fails than if it goes into the current spring retry loop.

lacking this feature directly: are there any alternative paths to the same result?

@tavin

This comment has been minimized.

@philwebb

This comment has been minimized.

@tavin

This comment has been minimized.

@keyzj
Copy link

keyzj commented Mar 25, 2022

Ops, shooting up legs in production...
Any updates on this? Maybe support API for configuration of this flag?

@keyzj
Copy link

keyzj commented Mar 25, 2022

Just not to be rude, i've come up with a workaround: you could use validation over your ConfigurationProperties and provide not valid default value for the required property.
For example:

Configuration properties class:

@Data
@Component
@Validated
@ConfigurationProperties("my-config-props")
public class MyConfigProps {
    @NotBlank
    private String host;
}

application.yaml:

my-config-props:
  host: ${MY_CONFIG_PROPS_HOST:}

@philwebb philwebb modified the milestones: 2.x, 3.x Aug 19, 2022
@rfelgent
Copy link

rfelgent commented Feb 14, 2024

hi,

just a reminder that this problem still exists with Spring Boot 3.2.

There is no easy configuration flag for enabling/disabling exception-behaviour for not resolvable ENV during configuration property binding. This missing feature causes some headaches/work around on our code side when implementing fail-fast failure detection

@philwebb maybe I am thinking to "simple", but why not adding a System-Property as feature toggle ?

Something like (my suggestion)

        public PropertySourcesPlaceholdersResolver(Iterable<PropertySource<?>> sources, PropertyPlaceholderHelper helper) {
		this.sources = sources;
		this.helper = (helper != null) ? helper : new PropertyPlaceholderHelper(SystemPropertyUtils.PLACEHOLDER_PREFIX,
				SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, 
                     Boolean.valueOf(System.getProperty("ignoreUnresolvablePlaceholders", "true"));
	}

instead of a hardcoded flag (current implementation)

        public PropertySourcesPlaceholdersResolver(Iterable<PropertySource<?>> sources, PropertyPlaceholderHelper helper) {
		this.sources = sources;
		this.helper = (helper != null) ? helper : new PropertyPlaceholderHelper(SystemPropertyUtils.PLACEHOLDER_PREFIX,
				SystemPropertyUtils.PLACEHOLDER_SUFFIX, SystemPropertyUtils.VALUE_SEPARATOR, true);
	}

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 15, 2024
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Mar 28, 2024
@LeMikaelF
Copy link

Is this open to contributions?

Second time I'm having an issue that I would have been caught with ignoreUnresolvablePlaceholders == true. Setting an empty string as a default in the placeholders (${MY_PROP:}) does not always result in a validation error (say with @NotEmpty), because you could have interpolated the value inside another string (my-topic.${app.env}).

Not having a fail-fast behaviour for this makes it harder to spot configuration errors at boot time, and more likely that they will cause production bugs further down the line.

@b-heimann-senacor
Copy link

I truly hope that this Fail Fast feature will be available someday, and I don’t understand why it isn’t just the standard behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: config-data Issues related to the configuration theme type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants