Skip to content

Commit

Permalink
Fix code review
Browse files Browse the repository at this point in the history
  • Loading branch information
PierreBtz committed Nov 17, 2019
1 parent 7af8c46 commit fd6df62
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 54 deletions.
20 changes: 15 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
<name>Audit Trail</name>
<version>2.6-SNAPSHOT</version>
<properties>
<jenkins.version>2.60.3</jenkins.version>
<jenkins.version>2.138.1</jenkins.version>
<java.level>8</java.level>
<configuration-as-code.version>1.23</configuration-as-code.version>
</properties>

<url>http://wiki.jenkins-ci.org/display/JENKINS/Audit+Trail+Plugin</url>
Expand All @@ -43,14 +42,12 @@
<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<classifier>tests</classifier>
<classifier>tests</classifier>AuditTrailFilter
<scope>test</scope>
</dependency>
</dependencies>
Expand Down Expand Up @@ -91,4 +88,17 @@
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.138.x</artifactId>
<version>3</version>
<scope>import</scope>
<type>pom</type>
</dependency>
</dependencies>
</dependencyManagement>

</project>
11 changes: 8 additions & 3 deletions src/main/java/hudson/plugins/audit_trail/AuditTrailFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,15 @@ public static void init() throws ServletException {
}

private void onRequest(String uri, String extra, String username) {
if (configuration.isStarted()) {
for (AuditLogger logger : configuration.getLoggers()) {
logger.log(uri + extra + " by " + username);
if (configuration != null) {
if (configuration.isStarted()) {
for (AuditLogger logger : configuration.getLoggers()) {
logger.log(uri + extra + " by " + username);
}
} else {
LOGGER.warning("Plugin configuration not properly injected, please report an issue to the Audit Trail Plugin");
}
}

}
}
14 changes: 6 additions & 8 deletions src/main/java/hudson/plugins/audit_trail/AuditTrailPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*/
package hudson.plugins.audit_trail;

import hudson.BulkChange;
import hudson.DescriptorExtensionList;
import hudson.Extension;

Expand All @@ -34,6 +33,8 @@
import jenkins.model.GlobalConfiguration;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -80,16 +81,11 @@ public AuditTrailPlugin() {
}

@Override
public boolean configure(StaplerRequest req, JSONObject formData) throws FormException {
public boolean configure(StaplerRequest req, JSONObject formData) {
// readResolve makes sure loggers is initialized, so it should never be null.
// TODO this should probably be moved somewhere else
loggers.forEach(AuditLogger::cleanUp);
try (BulkChange bc = new BulkChange(this)) {
req.bindJSON(this, formData);
bc.commit();
} catch (IOException e) {
throw new FormException("Failed to apply configuration", e, null);
}
req.bindJSON(this, formData);
applySettings();
return true;
}
Expand Down Expand Up @@ -141,6 +137,7 @@ boolean isStarted() {
/**
* @deprecated as of 2.6
**/
@Restricted(DoNotUse.class)
@Deprecated
public void onFinalized(Run run) {
LOGGER.warning("AuditTrailPlugin#onFinalized does nothing anymore, please update your script");
Expand All @@ -149,6 +146,7 @@ public void onFinalized(Run run) {
/**
* @deprecated as of 2.6
**/
@Restricted(DoNotUse.class)
@Deprecated
public void onFinalized(AbstractBuild build) {
LOGGER.warning("AuditTrailPlugin#onFinalized does nothing anymore, please update your script");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,73 @@
package hudson.plugins.audit_trail;

import hudson.ExtensionList;
import io.jenkins.plugins.casc.ConfigurationContext;
import io.jenkins.plugins.casc.ConfiguratorRegistry;
import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
import io.jenkins.plugins.casc.model.CNode;
import org.junit.ClassRule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static io.jenkins.plugins.casc.misc.Util.*;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.*;

/**
* Created by Pierre Beitz
* on 2019-07-20.
*/
public class ConfigurationAsCodeTest {

@ClassRule
@ConfiguredWithCode("jcasc.yml")
public static JenkinsConfiguredWithCodeRule r = new JenkinsConfiguredWithCodeRule();

@Issue("JENKINS-57232")
@Test
public void should_support_configuration_as_code() {
ExtensionList<AuditTrailPlugin> extensionList = r.jenkins.getExtensionList(AuditTrailPlugin.class);
AuditTrailPlugin plugin = extensionList.get(0);
assertEquals(".*/(?:configSubmit|doUninstall|doDelete|postBuildResult|enable|disable|cancelQueue|stop|toggleLogKeep|doWipeOutWorkspace|createItem|createView|toggleOffline|cancelQuietDown|quietDown|restart|exit|safeExit)", plugin.getPattern());
assertTrue(plugin.getLogBuildCause());
assertEquals(3, plugin.getLoggers().size());

//first logger
AuditLogger logger = plugin.getLoggers().get(0);
assertTrue(logger instanceof ConsoleAuditLogger);
assertEquals("test", ((ConsoleAuditLogger) logger).getLogPrefix());
assertEquals(ConsoleAuditLogger.Output.STD_OUT, ((ConsoleAuditLogger) logger).getOutput());
assertEquals("yyyy-MM-dd HH:mm:ss:SSS", ((ConsoleAuditLogger)logger).getDateFormat());

//second logger
logger = plugin.getLoggers().get(1);
assertTrue(logger instanceof LogFileAuditLogger);
assertEquals(10, ((LogFileAuditLogger)logger).getCount());
assertEquals(666, ((LogFileAuditLogger)logger).getLimit());
assertEquals("/log/location", ((LogFileAuditLogger)logger).getLog());

//third logger
logger = plugin.getLoggers().get(2);
assertEquals("jenkins", ((SyslogAuditLogger)logger).getAppName());
assertEquals("DAEMON", ((SyslogAuditLogger)logger).getFacility());
assertEquals("RFC_5424", ((SyslogAuditLogger)logger).getMessageFormat());
assertEquals("hostname", ((SyslogAuditLogger)logger).getMessageHostname());
assertEquals("syslog-server", ((SyslogAuditLogger)logger).getSyslogServerHostname());
assertEquals(514, ((SyslogAuditLogger)logger).getSyslogServerPort());
}
@ClassRule
@ConfiguredWithCode("jcasc.yml")
public static JenkinsConfiguredWithCodeRule r = new JenkinsConfiguredWithCodeRule();

@Issue("JENKINS-57232")
@Test
public void should_support_configuration_as_code() {
ExtensionList<AuditTrailPlugin> extensionList = r.jenkins.getExtensionList(AuditTrailPlugin.class);
AuditTrailPlugin plugin = extensionList.get(0);
assertEquals(".*/(?:configSubmit|doUninstall|doDelete|postBuildResult|enable|disable|cancelQueue|stop|toggleLogKeep|doWipeOutWorkspace|createItem|createView|toggleOffline|cancelQuietDown|quietDown|restart|exit|safeExit)", plugin.getPattern());
assertTrue(plugin.getLogBuildCause());
assertEquals(3, plugin.getLoggers().size());

//first logger
AuditLogger logger = plugin.getLoggers().get(0);
assertTrue(logger instanceof ConsoleAuditLogger);
assertEquals("test", ((ConsoleAuditLogger) logger).getLogPrefix());
assertEquals(ConsoleAuditLogger.Output.STD_OUT, ((ConsoleAuditLogger) logger).getOutput());
assertEquals("yyyy-MM-dd HH:mm:ss:SSS", ((ConsoleAuditLogger) logger).getDateFormat());

//second logger
logger = plugin.getLoggers().get(1);
assertTrue(logger instanceof LogFileAuditLogger);
assertEquals(10, ((LogFileAuditLogger) logger).getCount());
assertEquals(666, ((LogFileAuditLogger) logger).getLimit());
assertEquals("/log/location", ((LogFileAuditLogger) logger).getLog());

//third logger
logger = plugin.getLoggers().get(2);
assertEquals("jenkins", ((SyslogAuditLogger) logger).getAppName());
assertEquals("DAEMON", ((SyslogAuditLogger) logger).getFacility());
assertEquals("RFC_5424", ((SyslogAuditLogger) logger).getMessageFormat());
assertEquals("hostname", ((SyslogAuditLogger) logger).getMessageHostname());
assertEquals("syslog-server", ((SyslogAuditLogger) logger).getSyslogServerHostname());
assertEquals(514, ((SyslogAuditLogger) logger).getSyslogServerPort());
}

@Issue("JENKINS-57232")
@Test
public void should_support_configuration_export() throws Exception {
ConfiguratorRegistry registry = ConfiguratorRegistry.get();
ConfigurationContext context = new ConfigurationContext(registry);
CNode auditTrailAttribute = getUnclassifiedRoot(context).get("audit-trail");

String exported = toYamlString(auditTrailAttribute);

String expected = toStringFromYamlFile(this, "expected.yml");

assertThat(exported, is(expected));
}
}
18 changes: 18 additions & 0 deletions src/test/resources/hudson/plugins/audit_trail/expected.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
logBuildCause: true
loggers:
- console:
dateFormat: "yyyy-MM-dd HH:mm:ss:SSS"
logPrefix: "test"
output: STD_OUT
- logFile:
count: 10
limit: 666
log: "/log/location"
- syslog:
appName: "jenkins"
facility: "DAEMON"
messageFormat: "RFC_5424"
messageHostname: "hostname"
syslogServerHostname: "syslog-server"
syslogServerPort: 514
pattern: ".*/(?:configSubmit|doUninstall|doDelete|postBuildResult|enable|disable|cancelQueue|stop|toggleLogKeep|doWipeOutWorkspace|createItem|createView|toggleOffline|cancelQuietDown|quietDown|restart|exit|safeExit)"

0 comments on commit fd6df62

Please sign in to comment.