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

Properties migrator causes an application to fail to start if it tries to map a property whose metadata data entry contains an invalid configuration property name #32729

Closed
larsgrefer opened this issue Oct 14, 2022 · 7 comments
Labels
type: bug A general bug
Milestone

Comments

@larsgrefer
Copy link
Contributor

TL;DR: The spring-boot-configuration-processor should check for (or fix) invalid configuration property names found in META-INF/additional-spring-configuration-metadata.json at compile time.

We currently have the following situation:

  1. There is an invalid property name in META-INF/additional-spring-configuration-metadata.json of an external library.
  2. When the library is compiled, spring-boot-configuration-processor copies the invalid property name over to META-INF/spring-configuration-metadata.json.
  3. Our application depends on both the library and spring-boot-properties-migrator
  4. Our application fails to start with a org.springframework.boot.context.properties.source.InvalidConfigurationPropertyNameException.

Since the library is an external dependency out of our control, our only option is to exclude spring-boot-properties-migrator in our application until the metadata is fixed by the author of the external library.

This problem could have been prevented, if the invalid name were already caught in Step 2.
I therefore propose that spring-boot-configuration-processor should check META-INF/additional-spring-configuration-metadata.json for invalid property names, before copying them over to META-INF/spring-configuration-metadata.json. It can then either fail the compilation or convert the properties to their correct form.

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

Thanks for the suggestion. Unfortunately, I cannot reproduce the behavior that you have described.

I have some json describing the properties that looks like this:

{
  "groups": [
    {
      "name": "some",
      "type": "com.example.demo.SomeProperties",
      "sourceType": "com.example.demo.SomeProperties"
    }
  ],
  "properties": [
    {
      "name": "broken¢∞§¢#¢∞broken",
      "type": "java.lang.Boolean"
    },
    {
      "name": "some.two",
      "type": "java.lang.String",
      "sourceType": "com.example.demo.SomeProperties"
    },
    {
      "name": "some.one",
      "type": "java.lang.String",
      "sourceType": "com.example.demo.SomeProperties",
      "deprecated": true,
      "deprecation": {
        "replacement": "some.two"
      }
    }
  ],
  "hints": []
}

And some configuration that sets the deprecated property:

some.one=alpha

The app then starts successfully, warning me about the deprecated property and its mapping:

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::                (v2.7.5)

2022-11-17 17:47:22.196  INFO 72755 --- [           main] com.example.demo.Gh32729Application      : Starting Gh32729Application using Java 17.0.5 on wilkinsona-a01.vmware.com with PID 72755 (/Users/awilkinson/dev/workspaces/spring-projects/spring-boot/main/gh-32729/build/classes/java/main started by awilkinson in /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/main/gh-32729)
2022-11-17 17:47:22.197  INFO 72755 --- [           main] com.example.demo.Gh32729Application      : No active profile set, falling back to 1 default profile: "default"
2022-11-17 17:47:22.523  INFO 72755 --- [           main] com.example.demo.Gh32729Application      : Started Gh32729Application in 0.537 seconds (JVM running for 0.751)
2022-11-17 17:47:22.526  WARN 72755 --- [           main] o.s.b.c.p.m.PropertiesMigrationListener  : 
The use of configuration keys that have been renamed was found in the environment:

Property source 'Config resource 'class path resource [application.properties]' via location 'optional:classpath:/'':
        Key: some.one
                Line: 1
                Replacement: some.two


Each configuration key has been temporarily mapped to its replacement for your convenience. To silence this warning, please update your configuration to use the new keys.

Can you please provide a minimal example that shows the property with invalid characters causing a failure?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Nov 17, 2022
larsgrefer added a commit to larsgrefer/spring-boot-32729 that referenced this issue Nov 17, 2022
@larsgrefer
Copy link
Contributor Author

Thanks for looking into this.

Running ./gradlew app:bootRun on https://github.com/larsgrefer/spring-boot-32729 should reproduce the issue.

@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 Nov 17, 2022
@wilkinsona
Copy link
Member

Thanks, @larsgrefer. I've reproduced the problem now.

@wilkinsona
Copy link
Member

I don't think we can address this by failing compile time as it may result in compilation failures for things that are currently harmless. Without the properties migrator on the classpath, everything works nice with the IDE reporting that old.serverSSLPort has been deprecated and that new.server-ssl-port is its replacement. It's also possible to retrieve old.serverSSLPort from the environment using old.serverSSLPort or old.server-ssl-port. We could perhaps issue a warning at compile time that old.serverSSLPort isn't in the canonical form. I think we should also look at updating the properties migrator so that it doesn't cause a failure.

@wilkinsona wilkinsona changed the title Check for invalid property names at compile time Properties migrator causes an application to fail to start if it tries to map a property whose metadata data entry contains an invalid configuration property name Nov 17, 2022
@wilkinsona wilkinsona added this to the 2.6.x milestone Nov 17, 2022
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Nov 17, 2022
@wilkinsona
Copy link
Member

Possible change to the properties migrator that I'd like to discuss with @snicoll.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Nov 17, 2022
@larsgrefer
Copy link
Contributor Author

Allowing the properties migrator to work with non-canonical property keys would be Ideal.

This way it's easier to provide migration metadata for older (before spring boot) properties.

@snicoll
Copy link
Member

snicoll commented Nov 18, 2022

That change looks good @wilkinsona, thanks!

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

4 participants