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

fix: k8s:resource replaces template variables in provided fragments, k8s:helm doesn't. #410

Merged

Conversation

manusa
Copy link
Member

@manusa manusa commented Sep 28, 2020

Description

Fixes: #327

  • Moved logic from ResourceMojo to separate service
  • Fixed Resource generation, template placeholders are replaced only in aggregated YAML file (kubernetes.yml)
    not in individual resource files.
  • Placeholder/variable replacement can be turned off (jkube.interpolateTemplateParameters)
  • Fixed Helm chart generation, template placeholders are NOT replaced.
    Provided support for Parameters with no default value (values.yml) will only contain those which have a default.
    The rest of parameters can be specified with --set option
  • Added integration test to verify behavior for resource and helm YAML generation.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change (breaks current behavior because current behavior is buggy and inaccurate to spec)
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

@codecov
Copy link

codecov bot commented Sep 28, 2020

Codecov Report

Merging #410 into master will increase coverage by 0.35%.
The diff coverage is 55.97%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #410      +/-   ##
============================================
+ Coverage     36.29%   36.65%   +0.35%     
- Complexity     2344     2368      +24     
============================================
  Files           379      383       +4     
  Lines         18839    18867      +28     
  Branches       2756     2755       -1     
============================================
+ Hits           6838     6915      +77     
+ Misses        11181    11121      -60     
- Partials        820      831      +11     
Impacted Files Coverage Δ Complexity Δ
...kube/kit/config/service/ResourceServiceConfig.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../maven/plugin/enricher/DefaultEnricherManager.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...be/maven/plugin/mojo/build/AbstractDockerMojo.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...se/jkube/maven/plugin/mojo/build/ResourceMojo.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...e/kit/resource/service/DefaultResourceService.java 54.00% <54.00%> (ø) 7.00 <7.00> (?)
...ipse/jkube/kit/config/service/JKubeServiceHub.java 84.61% <66.66%> (-1.39%) 20.00 <1.00> (ø)
...g/eclipse/jkube/kit/resource/helm/HelmService.java 93.04% <66.66%> (-0.82%) 26.00 <2.00> (+1.00) ⬇️
.../eclipse/jkube/kit/resource/service/WriteUtil.java 90.00% <90.00%> (ø) 5.00 <5.00> (?)
...lipse/jkube/kit/resource/service/TemplateUtil.java 91.17% <91.17%> (ø) 10.00 <10.00> (?)
...ube/maven/plugin/mojo/build/AbstractJKubeMojo.java 73.52% <100.00%> (ø) 11.00 <1.00> (+1.00)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca5dbd2...983b8a9. Read the comment docs.

@manusa manusa force-pushed the fix/resource-placeholder-replacement branch from 6acadc8 to fd3da68 Compare September 28, 2020 09:33
@manusa manusa marked this pull request as ready for review September 28, 2020 09:47
@manusa manusa force-pushed the fix/resource-placeholder-replacement branch from fd3da68 to 69dafe9 Compare September 29, 2020 12:37
…k8s:helm doesn't.

- Moved logic from ResourceMojo to separate service
- Fixed Resource generation, template placeholders are replaced only in aggregated YAML file (kubernetes.yml)
  not in individual resource files.
- Placeholder/variable replacement can be turned off (jkube.interpolateTemplateParameters)
- Fixed Helm chart generation, template placeholders are NOT replaced.
  Provided support for Parameters with no default value (values.yml) will only contain those which have a default.
  The rest of parameters can be specified with `--set` option
- Added integration test to verify behavior for resource and helm YAML generation.

Signed-off-by: Marc Nuri <[email protected]>

WIP fix: k8s:resource replaces template variables in provided fragments, k8s:helm doesn't.

- Moved logic from ResourceMojo to separate service
- Fixed Resource generation, template placeholders are replaced only in aggregated YAML file (kubernetes.yml)
  not in individual resource files.
- Placeholder/variable replacement can be turned off (jkube.interpolateTemplateParameters)
- Fixed Helm chart generation, template placeholders are NOT replaced.
  Provided support for Parameters with no default value (values.yml) will only contain those which have a default.
  The rest of parameters can be specified with `--set` option
- Added integration test to verify behavior for resource and helm YAML generation.

Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa force-pushed the fix/resource-placeholder-replacement branch from 69dafe9 to 983b8a9 Compare September 30, 2020 11:28
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

41.9% 41.9% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_131) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@manusa manusa merged commit a80ecbd into eclipse-jkube:master Sep 30, 2020
@manusa manusa deleted the fix/resource-placeholder-replacement branch September 30, 2020 12:12
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.

k8s:helm should use values.yaml file
2 participants