Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/jenkins] Allow to enable OWASP Markup Formatter Plugin #10851

Merged
merged 1 commit into from
Jan 30, 2019
Merged

[stable/jenkins] Allow to enable OWASP Markup Formatter Plugin #10851

merged 1 commit into from
Jan 30, 2019

Conversation

375gnu
Copy link
Contributor

@375gnu 375gnu commented Jan 23, 2019

Signed-off-by: Hleb Valoshka [email protected]

What this PR does / why we need it:

If one works with ghprb jenkins plugin then data sent but github contains html snippets which are not parsed by default (see janinko/ghprb#140). To workaround this issue one needs to install OWASP Markup Formatter Plugin and to set markup formatter to Safe HTML.

Currently using the chart we can install the required plugins, but Safe HTML should be enabled manually. This PR fixes this inconvenience.

Which issue this PR fixes

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Jan 23, 2019
@maorfr
Copy link
Member

maorfr commented Jan 27, 2019

Hey @375gnu,

Happy to see another contribution from you!
Can this PR be simplified? I would expect no changes to _helpers.tpl for example. In addition, can the existing mechanism to install plugins be used?

What do you think?

@maorfr
Copy link
Member

maorfr commented Jan 27, 2019

/assign

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Jan 27, 2019
@375gnu
Copy link
Contributor Author

375gnu commented Jan 27, 2019

Hi @maorfr, I've simplified my pr a little. If you are ok with it, i'll squash commits.

My original idea was to make it fool proof that's why it was quite big.

@maorfr
Copy link
Member

maorfr commented Jan 27, 2019

Thanks for your reply and changes, much appreciated. It is looking much better!

I see that in config.yaml there is a mention of the plugin version. Can this be avoided? If not, we should at least document it.

In addition, I would prefer to not add the plugin installation, but rather document it instead, to avoid changing the default chart installation.

Will that be ok by you?

@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 28, 2019
@375gnu
Copy link
Contributor Author

375gnu commented Jan 28, 2019

Hi @maorfr , I've pushed a new commit addressing your requirements. If the pr looks good now for you I'll rebase it against the current master and squash into a single commit.

@maorfr
Copy link
Member

maorfr commented Jan 28, 2019

/ok-to-test
Looks good! Thanks for your cooperation :)
Feel free to squash if you want.

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 28, 2019
@375gnu
Copy link
Contributor Author

375gnu commented Jan 30, 2019

Hi @maorfr , I've squashed and rebased, could you merge it?

@maorfr
Copy link
Member

maorfr commented Jan 30, 2019

/lgtm

Thanks for contributing!

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 375gnu, maorfr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2019
@k8s-ci-robot k8s-ci-robot merged commit 9486e5d into helm:master Jan 30, 2019
@375gnu 375gnu deleted the html-formatter branch January 30, 2019 13:29
syedimam0012 pushed a commit to syedimam0012/charts that referenced this pull request Feb 1, 2019
wmcdona89 pushed a commit to wmcdona89/charts that referenced this pull request Aug 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants