feat(#937): Overrides environment variables when defined in resources

[lordofthejars]

Fix #937 Still no tests or applied to all enrichers because I am not sure about the approach. I think this is the cleanest way of doing it, but waiting for some feedback.

[codecov[bot]]

Codecov Report

Merging #1434 into master will increase coverage by 0.32%. The diff coverage is 37.14%.

@@             Coverage Diff              @@
##             master    #1434      +/-   ##
============================================
+ Coverage     34.63%   34.96%   +0.32%     
- Complexity     1045     1052       +7     
============================================
  Files           172      171       -1     
  Lines          9553     9536      -17     
  Branches       1635     1620      -15     
============================================
+ Hits           3309     3334      +25     
+ Misses         5869     5839      -30     
+ Partials        375      363      -12

[lordofthejars]

well actually this comes of a conversation we had about allowing enrichers to choose if they want to use it or not.

Missatge de Roland Huß [email protected] del dia dj., 13 de des. 2018 a les 14:43:

@rhuss commented on this pull request.

In enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DependencyEnricher.java https://github.com/fabric8io/fabric8-maven-plugin/pull/1434#discussion_r241403146 :

@@ -119,6 +120,12 @@ private void addArtifactsWithYaml(Set<URL> artifactSet, String dependencyYaml) {

 @Override
 public void adapt(final KubernetesListBuilder builder) {

•    getConfiguration().getResource().ifPresent(resourceConfig -> {
  

•        overrideEnvironmentVariables(builder, resourceConfig.getEnv()
  

•            .orElse(new HashMap<>()));
  

•    });
  

Really not sure why we need to duplicate that piece of code in every enricher. For me that looks like a smell.

Whats about a post processing step outside of all enricher, which just overrides the environment variables added by the enricher chain ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fabric8io/fabric8-maven-plugin/pull/1434#pullrequestreview-184665084, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcmYddBB-BaD32CDp5BSYPnTMFONNDmks5u4lltgaJpZM4YdfqU .

-- +----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+

[rhuss]

@lordofthejars yeah, I remember darkly. Do you have an example for an enricher who does the environment differently ? Not sure anymore whether we should guarantee a consistent behaviour over all enrichers or not.

[rhuss]

Test are failing because of a missing JMockit dependency. @lordofthejars could you please have a look ?