Skip to content

Conversation

@nguyensach
Copy link
Contributor

Fixes #25433

add isAutoDetectionEnabled method to determine that whether it is enabled that the platform is detected by looking for platform-specific environment variables.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 27, 2021
@nguyensach nguyensach force-pushed the fix_cloudPlatform_isActive branch 4 times, most recently from 32ce250 to dc90b92 Compare February 27, 2021 06:13
@nguyensach nguyensach force-pushed the fix_cloudPlatform_isActive branch from dc90b92 to de52e11 Compare February 27, 2021 08:28
@wilkinsona
Copy link
Member

Thanks for the pull request, @nguyensach. An alternative approach would be to change the condition. It could get the active CloudPlatform and see if it matches the required platform rather than checking if the required platform is active. I think I prefer what you've proposed here but I want to check with the team to see if the current behaviour of CloudPlatform is intentional. The follow code is in ConfigDataActivationContext:

private CloudPlatform deduceCloudPlatform(Environment environment, Binder binder) {
for (CloudPlatform candidate : CloudPlatform.values()) {
if (candidate.isEnforced(binder)) {
return candidate;
}
}
return CloudPlatform.getActive(environment);
}

It's making me wonder if there are some subtleties here that I'm overlooking. By calling isEnforced on each CloudPlatform and then falling back to getActive it seems to be allowing a CloudPlatform to be enforced ahead of an earlier one that would have been detected as active.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Mar 3, 2021
@nguyensach
Copy link
Contributor Author

nguyensach commented Mar 3, 2021

@wilkinsona Thanks for your response. I think that the code in ConfigDataActivationContext is right. When deduceCloudPlatform method is called, there is not actived profile.

* Create a new {@link ConfigDataActivationContext} instance before any profiles have
* been activated.

Therefore, All CloudPlatform#isEnforced(environment) will be false.

@philwebb philwebb added the status: on-hold We can't start working on this issue yet label Mar 10, 2021
@philwebb philwebb self-assigned this Mar 10, 2021
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Mar 10, 2021
@philwebb philwebb added type: bug A general bug and removed status: on-hold We can't start working on this issue yet status: waiting-for-triage An issue we've not yet triaged labels Apr 15, 2021
@philwebb philwebb added this to the 2.3.x milestone Apr 15, 2021
@philwebb philwebb changed the title Fix CloudPlatform.isActive CloudPlatform.isActive can return true when spring.main.cloud-platform is set to NONE Apr 15, 2021
philwebb pushed a commit that referenced this pull request Apr 15, 2021
Update `CloudPlatform.isActive` to back-off from detection when any
`spring.main.cloud-platform` property is set.

See gh-25455
@philwebb philwebb closed this in 930c637 Apr 15, 2021
@philwebb philwebb modified the milestones: 2.3.x, 2.3.10 Apr 15, 2021
@philwebb
Copy link
Member

Thanks very much @nguyensach for contributing to Spring Boot. This is now in 2.3.x, 2.4.x and 2.5.x

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

Successfully merging this pull request may close these issues.

Cannot enforce CloudPlatform.NONE via spring.main.cloud-platform

4 participants