-
Notifications
You must be signed in to change notification settings - Fork 282
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
feat: Allow suppress diff line output by regex #475
Conversation
e4d73c1
to
e1d52fc
Compare
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
@yxxhero @databus23 Do you have the time to look into it? |
@mumoshu @yxxhero @databus23 Please let me know, if I can assists here. |
@mumoshu @yxxhero @databus23 I would appreciate a review here. |
Co-authored-by: Yusuke Kuoka <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Jan-Otto Kröpke <[email protected]>
@mumoshu @yxxhero @databus23 I would appreciate a additional review here. |
Hi @mumoshu @yxxhero @databus23 I would appreciate a additional review here. |
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.
LGTM. Thanks a lot for your patience and contribution @jkroepke!!
Thanks a lot! @mumoshu do you plan a release wich include this change? |
@jkroepke Indeed! For transparency, the last thing we need before cutting the next release is to modify another feature we merged recently #458 somehow to make the dry-run=server an optional feature. That's to address @dudicoco's great insight shared in #449 (comment) |
when this feature will be released approximately? desperately waiting for it |
I too will sing your praises when this is released |
Hi @jkroepke. Can you please document the feature within the readme? |
Hi @dudicoco in our setup, we are using this:
it omits single lines. About multiple lines and regex, you have to be a regex pro, maybe this can work for you: https://regex101.com/r/OHEFVb/1 |
Thanks @jkroepke. The example you have provided did not work, I have even tried a more simple example which just captures the first two lines of the ports block and it also didn't work: So is the issue with the regex or just the fact that the new feature does not support multi lines regex? |
You are right, multi line is not supported. Reason: The underlaying library generated a line-by-line diff and the regex will be matched to each line. Technically, multi line regex is supported, but since its a line-by-line diff, it a regex across multiple lines will never work. The chances to support multiple are supper low because helm-diff have to merge the single lines first which would result into a too complex situation. |
thanks for info @jkroepke |
Does this affect only visible output or status codes/api results as well? I'm asking because Helmfile uses helm-diff under the hood to determine if a release is outdated and some Charts will always be re-deployed due to suboptimal design choices (e.g. random db passwords with no way to configure them via values). |
only visible output
but maybe |
This PR allow to suppress the diff report by using regex.
This option is more designed for power users and give them full control about the output.
There is a new diff option
--suppress-output-line-regex
which can be applied multiple time.If a line from the report matches the regex, the line gets removed from the report. If the diff entry has no deltas, the whole diffEntry (file) gets removed, except the headline
default, nginx, Deployment (apps) has changed:
Since
--suppress-output-line-regex
applied only to the output of an diff, the behavior of--detailed-exit-code
is not touch. If there are differences which are suppressed, the exit code remains still 2.Feature in action:
helm diff upgrade prometheus-node-exporter prometheus-community/prometheus-node-exporter --version 4.18.0
helm diff upgrade prometheus-node-exporter prometheus-community/prometheus-node-exporter --version 4.18.0 --suppress-output-line-regex "helm.sh/chart" --suppress-output-line-regex "version"