Skip to content
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

/actuator/configprops does not expose a property when second character is uppercase #13878

Closed
timtebeek opened this issue Jul 24, 2018 · 7 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@timtebeek
Copy link

timtebeek commented Jul 24, 2018

Hi! In migrating to Spring Boot 2 I came across a curious mismatch in our properties, illustrated in the following project & test: https://github.com/timtebeek/env-matching/blob/master/src/test/java/com/github/timtebeek/envmatching/EnvMatchingApplicationTests.java#L29

My application is nothing more than:

@Data
@SpringBootApplication
@ConfigurationProperties("sample")
@Validated
public class EnvMatchingApplication {

    private String someProp1;
    private String oAuthProp2;

    public static void main(String[] args) {
        SpringApplication.run(EnvMatchingApplication.class, args);
    }
}

But when I run my simple JUnit test:

@RunWith(SpringRunner.class)
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, properties = {
        "sample.someProp1=bla",
        "sample.oAuthProp2=bla",
})
public class EnvMatchingApplicationTests {
    @Autowired
    private TestRestTemplate restTemplate;

    @Test
    public void contextLoads() {
        ResponseEntity<String> entity = restTemplate.getForEntity("/actuator/configprops", String.class);
        assertThat(entity.getStatusCode()).isEqualByComparingTo(HttpStatus.OK);
        assertThat(entity.getBody()).contains("someProp1");
        assertThat(entity.getBody()).contains("oAuthProp2");
    }

}

It fails on the final assertion. So sample.someProp1 is matched perfectly fine, whereas sample.oAuthProp2 is not matched. In our case this leads to misconfiguration(!) and some confusion. I've found the workaround to be to lowercase the property, but that's quite surprising to me. Can you enlighten me if I'm correct in having assumed this would work?

I'm aware there were substantial changes to the relaxed binding in Spring Boot 2, but never expected the second character being uppercase to be a problem at all. An uppercase letter as third character also work fine, so really weird behavior here.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 24, 2018
@snicoll
Copy link
Member

snicoll commented Jul 24, 2018

So sample.someProp1 is matched perfectly fine, whereas sample.oAuthProp2 is not matched. In our case this leads to misconfiguration(!) and some confusion.

I can confirm that the configprops endpoint doesn't show the property, which is odd so we have to do something there.

Can you please clarify what you mean by "misconfiguration (!)". I can see that the setter is called with the proper value once I've removed lombok from the picture (and I can add a debug point in the setter).

The canonical name of that property is sample.o-auth-prop2

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jul 24, 2018
@timtebeek
Copy link
Author

Thanks for the quick reply; You're right in that the correct value is indeed stored in the app class fields. I'd correlated not seeing them in configprops as cause of the issue we're having: a default value we set in application.yml for sample.oAuthProp2 is not overridden in our deployment env by a environment variable sample_oAuthProp2. However, I'm unable to replicate this in my minimal project, so that might have a different cause still. I'll explore further if the two are related or separate issues (tomorrow).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 24, 2018
@snicoll
Copy link
Member

snicoll commented Jul 24, 2018

That’s not how an environment variable should be formatted. Please review the documentation.

Before opening a new issue, let's figure out what this one is about. If I turns out your project is working fine we can create a dedicated one for the configProps mismatch. Thanks

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 24, 2018
@timtebeek
Copy link
Author

Thanks for the hint on enviroment variable formatting; I'd looked towards the Relaxed Binding 2.0 Wiki page which reads:

Environment variables are bound by lowercasing and replacing _ with ..
For example: SPRING_JPA_DATABASEPLATFORM=mysql results in the property spring.jpa.databaseplatform=mysql.

I'd took that to mean as that any mixed case environment variable is lowercased and only then matched against @ConfigurationProperties annotated classes, but it sounds like that must have been an erroneous interpretation on my part. (we have a large/legacy configuration project using the mixed case format).

From the documentation link you gave it seems I should uppercase all my environment properties, correct?

If confirmed (tomorrow, for lack of access now) that would reduce this issue down to something along the lines of "actuator configprops fails to show properties where second character is uppercase"; a weird one, that sent me down the wrong path debugging what might still be a configuration migration issue on our part.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 24, 2018
@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 25, 2018
@timtebeek
Copy link
Author

Sorry for the delay in responding; it does appear as though the correct values are set, reducing this issue down to merely the properties shown in /actuator/configprops. Which is still confusing when debugging, but much less an issue then incorrect values would be.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 27, 2018
@snicoll snicoll changed the title Config property mismatch when second character is uppercase /actuator/configprops does not expose a property when second character is uppercase Jul 27, 2018
@snicoll snicoll added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jul 27, 2018
@snicoll snicoll added this to the 1.5.x milestone Jul 27, 2018
@snicoll
Copy link
Member

snicoll commented Jul 27, 2018

Thanks for the feedback @timtebeek

@snicoll
Copy link
Member

snicoll commented Aug 7, 2018

So the bug/limitation is in the way we configure the ObjectMapper, GenericSerializerModifier is not able to detect the setter properly.

As a result, ObjectMapper#convertValue(bean, Map.class) ignores oAuthProp2.

There might be a limitation in Jackson as well, see this issue

snicoll added a commit to snicoll/spring-boot that referenced this issue Aug 7, 2018
@snicoll snicoll self-assigned this Aug 7, 2018
@snicoll snicoll modified the milestones: 1.5.x, 2.0.5 Aug 8, 2018
@snicoll snicoll closed this as completed in fbf3c48 Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants