Skip to content

Fix 1310 for ConfigMaps#1332

Merged
ryanjbaxter merged 63 commits intospring-cloud:mainfrom
wind57:fix-1310
May 2, 2023
Merged

Fix 1310 for ConfigMaps#1332
ryanjbaxter merged 63 commits intospring-cloud:mainfrom
wind57:fix-1310

Conversation

@wind57
Copy link
Copy Markdown
Contributor

@wind57 wind57 commented Apr 30, 2023

No description provided.

wind57 and others added 30 commits December 4, 2021 07:59
else {
composite.addFirstPropertySource(new MapPropertySource(name, map));
LOG.debug("will add file-based property source : " + name);
composite.addFirstPropertySource(new MountConfigMapPropertySource(name, map));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is 1/2 the fix, provide a well known class so that we can later check for it.

/**
* @author wind57
*/
public final class MountConfigMapPropertySource extends MapPropertySource {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a generic MapPropertySource, use a class that can be later checked easily via instanceOf

else if (sourceClass.isInstance(source)) {
managedSources.add(sourceClass.cast(source));
}
else if (source instanceof MountConfigMapPropertySource mountConfigMapPropertySource) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other 1/2 of the fix. Also a bit down in the code also, for the bootstrap case


/**
* <pre>
* - we have "spring.config.import: kubernetes", which means we will 'locate' property sources
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have 4 of these integration tests: with config-data and bootstrap for fabric8 and k8s-client.

@wind57 wind57 changed the title Fix 1310 Fix 1310 for ConfigMaps May 1, 2023
@wind57 wind57 marked this pull request as ready for review May 1, 2023 08:00
@wind57
Copy link
Copy Markdown
Contributor Author

wind57 commented May 1, 2023

@ryanjbaxter I have split the fix into two parts: one for ConfigMaps and one for Secrets (will work on it after these PRs). I also assume this needs to be fixed in 2.1.x and main, as such two PRs. Thank you for looking into it.

@ryanjbaxter ryanjbaxter linked an issue May 1, 2023 that may be closed by this pull request
@ryanjbaxter ryanjbaxter merged commit 944c7d7 into spring-cloud:main May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to listen for file changes

3 participants