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 does not detect properties of Map type that are marked as deprecated #27854

Closed
jvillarb opened this issue Sep 2, 2021 · 8 comments
Labels
type: bug A general bug
Milestone

Comments

@jvillarb
Copy link

jvillarb commented Sep 2, 2021

If we set a Map property as deprecated, because it is replaced or removed, the Spring Boot Properties Migrator module, when scanning the application environment and printing diagnostics, does not detect these types of properties.

For example, if we have the custom.logging.log-level property of type java.util.Map defined in the the metadata file as follows:

{
  "name": "custom.logging.log-level",
  "type": "java.util.Map<java.lang.String,java.lang.String>",
  "description": "",
  "deprecation": {
	"level": "error",
	"replacement": "logging.level"
  }
}

and set in the application.yml:

custom:
  logging:
      log-level:
          xx.xxxx.xxxx: INFO

When the spring-boot-properties-migrator module is included and the application is run, the generated report does not show that this property has been replaced by "logging.level".

The same problem can also be encountered with the management.health.status.http-mapping property of the spring-boot-actuator-autoconfigure library:

{
  "name": "management.health.status.http-mapping",
  "type": "java.util.Map<java.lang.String,java.lang.Integer>",
  "sourceType": "org.springframework.boot.actuate.autoconfigure.health.HealthIndicatorProperties",
  "deprecated": true,
  "deprecation": {
	"replacement": "management.endpoint.health.status.http-mapping"
  }
}
spring:
  http:
    encoding:
      charset: UTF-8

management:
  health:
    status:
      http-mapping:
        down: 500
        out_of_service: 503
        warning: 500

The displayed result does not show the deprecated property management.health.status.http-mapping.

[WARN ] 2021-09-02 16:35:49.601 [main]     PropertiesMigrationListener - 
The use of configuration keys that have been renamed was found in the environment:

Property source 'applicationConfig: [classpath:/config/application.yml]':
	Key: spring.http.encoding.charset
		Line: 9
		Replacement: server.servlet.encoding.charset


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.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 2, 2021
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 9, 2021
@wilkinsona wilkinsona added this to the 2.4.x milestone Sep 9, 2021
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.5.x Nov 17, 2021
@dominiccroce
Copy link

@wilkinsona @jvillarb Starting work on this issue (see fork here). Currently using the above example as a test case, although it seems incomplete. What is the metadata for logging.level? Is it explicitly defined elsewhere in the metadata file, or should it be inferred the both log-level and level are coming from the logging map, so we can infer the new metadata is the same type as the old metadata? That second case seems specific for when deprecation is within the same map as it is in this case. Is my understanding correct that logging is the map and we are trying to rename the field log-level to level?

Currently, my test case is giving a report of

The use of configuration keys that are no longer supported was found in the environment:

Property source 'custom':
	Key: custom.logging.log-level
		Reason: No metadata found for replacement key 'logging.level'

Is this the report that seen in the original issue? If not, what was the report? Just want to make sure I am truly reproducing the original issue. Thinking desired report looks like:

The use of configuration keys that have been renamed was found in the environment:

Property source 'custom':
	Key: custom.logging.log-level
		Replacement: custom.logging.level

@snicoll
Copy link
Member

snicoll commented Nov 22, 2021

@dominiccroce thanks for looking into this. I had a quick look at your fork and I don't think that address the issue that's reported here. The problem is that we have a metadata entry for the map, but not for the individual entries (as they are potentially infinite).

Taking back the example above, management.health.status.http-mapping is a deprecated Map property but management.health.status.http-mapping isn't the key. Rather management.health.status.http-mapping.down (for instance) is.

This issue is about checking if the prefix of a key matches a deprecated map entry and then add it to the report. There are multiple ways of doing that and I can't say for sure which one is best. Let me know if you want to pursue this!

@dominiccroce
Copy link

dominiccroce commented Nov 22, 2021

Hi @snicoll Thanks for the response. I think makes sense, the map itself is deprecated but individual entries aren't matching since we only look for exact match but not prefix match. I will continue to look at this and adjust my test case accordingly.

Just to confirm, this means that the individual map entries are not explicitly defined in the metadata file, just the map itself and the renamed map will be defined? Also to confirm, this means the key will not be changing, only the map itself is?

@snicoll
Copy link
Member

snicoll commented Nov 23, 2021

Yep, that's right. Thanks for following up and let us know if you have other questions.

@dominiccroce
Copy link

@snicoll I believe I have reproduced the issue in a unit test and now said test is passing. Please see attached PR and we can discuss further there if you have any concerns with the test or implementation.

@dominiccroce
Copy link

@snicoll No rush but just curious on when I can get some feedback on my PR?

@snicoll
Copy link
Member

snicoll commented Dec 2, 2021

@dominiccroce not sure what kind of answer you expect besides "when time permits". I am working on several projects and we're all pretty busy at the moment.

@dominiccroce
Copy link

@snicoll Thanks for the response. No worries, this is my first contribution to this project so was just unsure on what to expect for timeline on reviews.

@wilkinsona wilkinsona changed the title Spring Boot Properties Migrator - Not detect properties of Map type that are marked as deprecated Properties Migrator does not detect properties of Map type that are marked as deprecated Mar 23, 2022
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x May 19, 2022
@wilkinsona wilkinsona modified the milestones: 2.6.x, 2.7.x Nov 24, 2022
@mhalbritter mhalbritter modified the milestones: 2.7.x, 2.7.9 Jan 24, 2023
krenson pushed a commit to krenson/test-push that referenced this issue Mar 15, 2023
…ot-starter-parent from 2.7.8 to 2.7.9 (patch)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `2.7.8` -> `2.7.9` |

---

### Release Notes

<details>
<summary>spring-projects/spring-boot</summary>

### [`v2.7.9`](https://github.com/spring-projects/spring-boot/releases/tag/v2.7.9)

[Compare Source](spring-projects/spring-boot@v2.7.8...v2.7.9)

#### 🐞 Bug Fixes

-   Maven Plugin's PropertiesMergingResourceTransformer closes InputStream when it should not do so [#&#8203;34063](spring-projects/spring-boot#34063)
-   Actuator Health web endpoint broken with Gson and Java 17 [#&#8203;34030](spring-projects/spring-boot#34030)
-   Dependency management for Mongo's Java Driver is incomplete [#&#8203;33941](spring-projects/spring-boot#33941)
-   Using devtools with Reactive application results in slower restarts [#&#8203;33855](spring-projects/spring-boot#33855)
-   Spies are not reset after test execution when using `@SpyBean` [#&#8203;33830](spring-projects/spring-boot#33830)
-   Properties Migrator does not detect properties of Map type that are marked as deprecated [#&#8203;27854](spring-projects/spring-boot#27854)

#### 📔 Documentation

-   Updated documentation for `@ConfigurationProperties` bean naming rules [#&#8203;34029](spring-projects/spring-boot#34029)
-   Restore "Use Jedis Instead of Lettuce" how-to documentation [#&#8203;33994](spring-projects/spring-boot#33994)
-   Add Redis application properties example [#&#8203;33965](spring-projects/spring-boot#33965)
-   Use Maven Central for release downloads in CLI installation documentation [#&#8203;33962](spring-projects/spring-boot#33962)
-   Actuator section is missing from documentation overview [#&#8203;33932](spring-projects/spring-boot#33932)
-   Add Javadoc since to OperationParameter.getAnnotation() [#&#8203;33914](spring-projects/spring-boot#33914)
-   Document additional configuration that is required for spring.mvc.throw-exception-if-no-handler-found=true to be effective [#&#8203;31660](spring-projects/spring-boot#31660)

#### 🔨 Dependency Upgrades

-   Upgrade to ActiveMQ 5.16.6 [#&#8203;34238](spring-projects/spring-boot#34238)
-   Upgrade to Byte Buddy 1.12.23 [#&#8203;34239](spring-projects/spring-boot#34239)
-   Upgrade to Dropwizard Metrics 4.2.16 [#&#8203;34240](spring-projects/spring-boot#34240)
-   Upgrade to Elasticsearch 7.17.9 [#&#8...
krenson pushed a commit to krenson/test-push that referenced this issue Mar 15, 2023
…ot-starter-parent from 2.7.8 to 2.7.9 (patch)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `2.7.8` -> `2.7.9` |

---

### Release Notes

<details>
<summary>spring-projects/spring-boot</summary>

### [`v2.7.9`](https://github.com/spring-projects/spring-boot/releases/tag/v2.7.9)

[Compare Source](spring-projects/spring-boot@v2.7.8...v2.7.9)

#### 🐞 Bug Fixes

-   Maven Plugin's PropertiesMergingResourceTransformer closes InputStream when it should not do so [#&#8203;34063](spring-projects/spring-boot#34063)
-   Actuator Health web endpoint broken with Gson and Java 17 [#&#8203;34030](spring-projects/spring-boot#34030)
-   Dependency management for Mongo's Java Driver is incomplete [#&#8203;33941](spring-projects/spring-boot#33941)
-   Using devtools with Reactive application results in slower restarts [#&#8203;33855](spring-projects/spring-boot#33855)
-   Spies are not reset after test execution when using `@SpyBean` [#&#8203;33830](spring-projects/spring-boot#33830)
-   Properties Migrator does not detect properties of Map type that are marked as deprecated [#&#8203;27854](spring-projects/spring-boot#27854)

#### 📔 Documentation

-   Updated documentation for `@ConfigurationProperties` bean naming rules [#&#8203;34029](spring-projects/spring-boot#34029)
-   Restore "Use Jedis Instead of Lettuce" how-to documentation [#&#8203;33994](spring-projects/spring-boot#33994)
-   Add Redis application properties example [#&#8203;33965](spring-projects/spring-boot#33965)
-   Use Maven Central for release downloads in CLI installation documentation [#&#8203;33962](spring-projects/spring-boot#33962)
-   Actuator section is missing from documentation overview [#&#8203;33932](spring-projects/spring-boot#33932)
-   Add Javadoc since to OperationParameter.getAnnotation() [#&#8203;33914](spring-projects/spring-boot#33914)
-   Document additional configuration that is required for spring.mvc.throw-exception-if-no-handler-found=true to be effective [#&#8203;31660](spring-projects/spring-boot#31660)

#### 🔨 Dependency Upgrades

-   Upgrade to ActiveMQ 5.16.6 [#&#8203;34238](spring-projects/spring-boot#34238)
-   Upgrade to Byte Buddy 1.12.23 [#&#8203;34239](spring-projects/spring-boot#34239)
-   Upgrade to Dropwizard Metrics 4.2.16 [#&#8203;34240](spring-projects/spring-boot#34240)
-   Upgrade to Elasticsearch 7.17.9 [#&#8...
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

6 participants