-
Notifications
You must be signed in to change notification settings - Fork 202
Allows user to define Secret from annotation #1498
Conversation
Signed-off-by: bohmber <bommel@apache.org>
Codecov Report
@@ Coverage Diff @@
## master #1498 +/- ##
===========================================
+ Coverage 35.85% 35.96% +0.1%
- Complexity 1109 1116 +7
===========================================
Files 177 178 +1
Lines 10001 10027 +26
Branches 1658 1660 +2
===========================================
+ Hits 3586 3606 +20
- Misses 5993 5998 +5
- Partials 422 423 +1 |
Codecov Report
@@ Coverage Diff @@
## master #1498 +/- ##
============================================
+ Coverage 35.85% 36.02% +0.16%
- Complexity 1109 1117 +8
============================================
Files 177 178 +1
Lines 10001 10027 +26
Branches 1658 1660 +2
============================================
+ Hits 3586 3612 +26
+ Misses 5993 5991 -2
- Partials 422 424 +2 |
final Set<Map.Entry<String, String>> entries = annotations.entrySet(); | ||
final Map<String, String> secretFileLocations = new HashMap<>(); | ||
|
||
for(Iterator<Map.Entry<String, String>> it = entries.iterator(); it.hasNext(); ) { |
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.
Hey @bohmber why not just use
for (Map.Entry<String, String> entry : annotations.entrySet( ) )
?
The FMP code base uses this way of iteration everywhere, so it would look uniform with the codebase if you change it to this style.
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.
It's just a modified copy of the ConfigMapEnricher and i think the iterator is needed for the remove a few lines later
it.remove();
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.
oh i see
...er/standard/src/test/java/io/fabric8/maven/enricher/standard/FileDataSecretEnricherTest.java
Outdated
Show resolved
Hide resolved
Really appreciate you adding test and documentation with your code. |
Shouldn't the annotation get deleted/removed after it is 'enriched' into a proper secret? I'm not a kubernetes/openshift expert in the matter though. Correct me if I'm wrong here. Also, Changelog entry missing. |
@dev-gaur The annotation is deleted see my comment about the iterator and there is an assert in the unit test for it. |
Signed-off-by: bohmber <bommel@apache.org>
@dev-gaur I can't add the change log entry before the pull request is created. I need the number and adding the change log entry early is causing more work on my side because i have to fix merge conflicts in the change log file. I can add the change log entry before the pull request will be merged |
Is |
aah missed that. We'll handle the merge conflicts before merging. You can go ahead with the changelog entry. |
Signed-off-by: bohmber <bommel@apache.org>
@rohanKanojia changelog entry added |
Similar to fmp-configmap-file enricher for annotation
Signed-off-by: bohmber bommel@apache.org