-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix 1310 for ConfigMaps #1332
Fix 1310 for ConfigMaps #1332
Conversation
Configure Renovate
@@ -139,7 +140,8 @@ private void addPropertySourceIfNeeded(Function<String, Map<String, Object>> con | |||
LOG.warn("Property source: " + name + "will be ignored because no properties could be found"); | |||
} | |||
else { | |||
composite.addFirstPropertySource(new MapPropertySource(name, map)); | |||
LOG.debug("will add file-based property source : " + name); | |||
composite.addFirstPropertySource(new MountConfigMapPropertySource(name, map)); |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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
@@ -84,14 +85,24 @@ public static <S extends PropertySource<?>> List<S> findPropertySources(Class<S> | |||
else if (sourceClass.isInstance(source)) { | |||
managedSources.add(sourceClass.cast(source)); | |||
} | |||
else if (source instanceof MountConfigMapPropertySource mountConfigMapPropertySource) { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
@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 |
No description provided.