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

use modern api in ConsoleLogFilter to allow for pipeline jobs to be filtered globally #18

Merged
merged 2 commits into from
May 31, 2018

Conversation

mwinter69
Copy link
Contributor

When enabled globally it should work with all types of Jobs, also pipelines. Depends on JENKINS-45693 or that JENKINS-38381 implements this.

When enabled globally it should work with all types of Jobs, also
pipelines once JENKINS-45693 is fixed.
@oleg-nenashev oleg-nenashev self-requested a review March 15, 2018 22:25
@@ -69,7 +43,9 @@
* @author Jason Antman jason@jasonantman.com
*/
@Extension
public class MaskPasswordsConsoleLogFilter extends ConsoleLogFilter {
public class MaskPasswordsConsoleLogFilter extends ConsoleLogFilter implements Serializable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it really need to be serializable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not going to work, since you will be calling MaskPasswordsConfig.getInstance() from the agent JVM and this will at best return junk.

To make this actually function, you would need to create a writeReplace method which captures passwords and regexes in a nontransient instance field, or something similar.

@oleg-nenashev
Copy link
Member

Agreed

@gregoryboue
Copy link

Hi,

I'am really interesting by this PR, when it will be released ?

Thanks you

@oleg-nenashev
Copy link
Member

Thanks for the ping, releasing

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.

4 participants