From 371e27bf3382e7418d95c4fec658fae4700d285d Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Mon, 11 Dec 2017 22:57:36 +0100 Subject: [PATCH 01/17] properly get displayname of node use the executor of the build to get the node. builtOn is set too late for us to get the node and we end up using master --- .../logstash/persistence/BuildData.java | 3 +- .../plugins/logstash/LogstashWriterTest.java | 12 +++++-- .../logstash/persistence/BuildDataTest.java | 32 ++++++++++++++----- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java index abb8a5be..88f33f0b 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java @@ -25,6 +25,7 @@ package jenkins.plugins.logstash.persistence; import hudson.model.Action; +import hudson.model.Computer; import hudson.model.Environment; import hudson.model.Result; import hudson.model.AbstractBuild; @@ -138,7 +139,7 @@ public TestData(Action action) { public BuildData(AbstractBuild build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Node node = build.getBuiltOn(); + Node node = build.getExecutor().getOwner().getNode(); if (node == null) { buildHost = "master"; buildLabel = "master"; diff --git a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java index 8e643e39..3d191a83 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java @@ -2,6 +2,8 @@ import hudson.EnvVars; import hudson.model.AbstractBuild; +import hudson.model.Computer; +import hudson.model.Executor; import hudson.model.Project; import hudson.model.Result; import hudson.model.TaskListener; @@ -75,6 +77,9 @@ String getJenkinsUrl() { @Mock BuildData mockBuildData; @Mock TaskListener mockListener; + @Mock Computer mockComputer; + @Mock Executor mockExecutor; + @Captor ArgumentCaptor> logLinesCaptor; @@ -85,7 +90,6 @@ public void before() throws Exception { when(mockBuild.getDisplayName()).thenReturn("LogstashNotifierTest"); when(mockBuild.getProject()).thenReturn(mockProject); when(mockBuild.getParent()).thenReturn(mockProject); - when(mockBuild.getBuiltOn()).thenReturn(null); when(mockBuild.getNumber()).thenReturn(123456); when(mockBuild.getTimestamp()).thenReturn(new GregorianCalendar()); when(mockBuild.getRootBuild()).thenReturn(mockBuild); @@ -97,6 +101,10 @@ public void before() throws Exception { when(mockBuild.getLog(3)).thenReturn(Arrays.asList("line 1", "line 2", "line 3", "Log truncated...")); when(mockBuild.getLog(Integer.MAX_VALUE)).thenReturn(Arrays.asList("line 1", "line 2", "line 3", "line 4")); when(mockBuild.getEnvironment(null)).thenReturn(new EnvVars()); + when(mockBuild.getExecutor()).thenReturn(mockExecutor); + when(mockExecutor.getOwner()).thenReturn(mockComputer); + when(mockComputer.getNode()).thenReturn(null); + when(mockTestResultAction.getTotalCount()).thenReturn(0); when(mockTestResultAction.getSkipCount()).thenReturn(0); @@ -142,7 +150,7 @@ public void constructorSuccess() throws Exception { verify(mockBuild).getDescription(); verify(mockBuild).getUrl(); verify(mockBuild).getAction(AbstractTestResultAction.class); - verify(mockBuild).getBuiltOn(); + verify(mockBuild).getExecutor(); verify(mockBuild, times(2)).getNumber(); verify(mockBuild).getTimestamp(); verify(mockBuild, times(4)).getRootBuild(); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java index 51dfb09b..c623be66 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java @@ -28,8 +28,10 @@ import hudson.EnvVars; import hudson.model.AbstractBuild; +import hudson.model.Computer; import hudson.model.Environment; import hudson.model.EnvironmentList; +import hudson.model.Executor; import hudson.model.Node; import hudson.model.Project; import hudson.model.Result; @@ -61,6 +63,8 @@ public class BuildDataTest { @Mock Environment mockEnvironment; @Mock Date mockDate; @Mock TaskListener mockListener; + @Mock Computer mockComputer; + @Mock Executor mockExecutor; @Before @@ -97,6 +101,8 @@ public void before() throws Exception { when(mockRootProject.getFullName()).thenReturn("parent/RootBuildDataTest"); when(mockDate.getTime()).thenReturn(60L); + when(mockBuild.getExecutor()).thenReturn(mockExecutor); + when(mockExecutor.getOwner()).thenReturn(mockComputer); } @After @@ -124,7 +130,7 @@ private void verifyMocks() throws Exception verify(mockBuild).getStartTimeInMillis(); verify(mockBuild).getUrl(); verify(mockBuild).getAction(AbstractTestResultAction.class); - verify(mockBuild).getBuiltOn(); + verify(mockBuild).getExecutor(); verify(mockBuild).getNumber(); verify(mockBuild).getTimestamp(); verify(mockBuild, times(4)).getRootBuild(); @@ -133,6 +139,8 @@ private void verifyMocks() throws Exception verify(mockBuild).getEnvironments(); verify(mockBuild).getEnvironment(mockListener); + verify(mockExecutor).getOwner(); + verify(mockRootProject).getName(); verify(mockRootProject).getFullName(); @@ -150,9 +158,15 @@ private void verifyTestResultActions() { verify(mockTestResultAction, times(1)).getFailedTests(); } + private void verifiyNodeActions(int labelCount) { + verify(mockComputer).getNode(); + verify(mockNode, times(2)).getDisplayName(); + verify(mockNode, times(labelCount)).getLabelString(); + } + @Test public void constructorSuccessBuiltOnNull() throws Exception { - when(mockBuild.getBuiltOn()).thenReturn(null); + when(mockComputer.getNode()).thenReturn(null); // Unit under test BuildData buildData = new BuildData(mockBuild, mockDate, mockListener); @@ -170,7 +184,7 @@ public void constructorSuccessBuiltOnNull() throws Exception { @Test public void constructorSuccessBuiltOnMaster() throws Exception { - when(mockBuild.getBuiltOn()).thenReturn(mockNode); + when(mockComputer.getNode()).thenReturn(mockNode); when(mockNode.getDisplayName()).thenReturn("Jenkins"); when(mockNode.getLabelString()).thenReturn(""); @@ -187,11 +201,12 @@ public void constructorSuccessBuiltOnMaster() throws Exception { verifyMocks(); verifyTestResultActions(); + verifiyNodeActions(1); } @Test public void constructorSuccessBuiltOnSlave() throws Exception { - when(mockBuild.getBuiltOn()).thenReturn(mockNode); + when(mockComputer.getNode()).thenReturn(mockNode); when(mockNode.getDisplayName()).thenReturn("Test Slave 01"); when(mockNode.getLabelString()).thenReturn("Test Slave"); @@ -208,6 +223,7 @@ public void constructorSuccessBuiltOnSlave() throws Exception { verifyMocks(); verifyTestResultActions(); + verifiyNodeActions(2); } @Test @@ -252,8 +268,7 @@ public void constructorSuccessWithEnvVars() throws Exception { when(mockBuild.getEnvironments()).thenReturn(new EnvironmentList(Arrays.asList(mockEnvironment))); when(mockBuild.getBuildVariables()).thenReturn(new HashMap()); - when(mockBuild.getBuiltOn()).thenReturn(mockNode); - + when(mockComputer.getNode()).thenReturn(mockNode); when(mockNode.getDisplayName()).thenReturn("Jenkins"); when(mockNode.getLabelString()).thenReturn(""); @@ -289,6 +304,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { verifyMocks(); verifyTestResultActions(); + verifiyNodeActions(1); } @Test // JENKINS-41324 @@ -297,8 +313,7 @@ public void constructorSuccessWithChangedEnvVars() throws Exception { when(mockBuild.getBuildVariables()).thenReturn(new HashMap()); when(mockBuild.getSensitiveBuildVariables()).thenReturn(new HashSet()); - when(mockBuild.getBuiltOn()).thenReturn(mockNode); - + when(mockComputer.getNode()).thenReturn(mockNode); when(mockNode.getDisplayName()).thenReturn("Jenkins"); when(mockNode.getLabelString()).thenReturn(""); @@ -327,6 +342,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { verifyMocks(); verifyTestResultActions(); + verifiyNodeActions(1); } @Test From 67bc346e4aa64e41df18e9211afd6db1ef532b13 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Sat, 16 Dec 2017 23:23:48 +0100 Subject: [PATCH 02/17] displayname of node in pipeline properly get the node when BuildData is initilaized from a pipeline pump mockito to latest version add integration test, that use Jenkins test harness --- pom.xml | 19 +- .../AbstractLogstashIndexerDao.java | 4 +- .../logstash/persistence/BuildData.java | 9 +- .../logstash/LogstashIntegrationTest.java | 163 ++++++++++++++++++ .../logstash/LogstashNotifierTest.java | 8 +- .../plugins/logstash/LogstashWriterTest.java | 4 +- .../logstash/persistence/BuildDataTest.java | 3 +- 7 files changed, 191 insertions(+), 19 deletions(-) create mode 100644 src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java diff --git a/pom.xml b/pom.xml index 518f0589..e9da92c5 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ org.jenkins-ci.plugins plugin - 1.599 + 1.608 @@ -65,8 +65,6 @@ redis.clients jedis 2.6.0 - jar - compile @@ -88,8 +86,7 @@ org.mockito mockito-core - 2.1.0 - jar + 2.10.0 test @@ -127,6 +124,18 @@ structs 1.2 + + org.powermock + powermock-api-mockito2 + 2.0.0-beta.5 + test + + + org.powermock + powermock-module-junit4 + 2.0.0-beta.5 + test + diff --git a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java index 25bd93c6..c17b61ce 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java @@ -37,14 +37,14 @@ * @author Rusty Gerard * @since 1.0.0 */ -abstract class AbstractLogstashIndexerDao implements LogstashIndexerDao { +public abstract class AbstractLogstashIndexerDao implements LogstashIndexerDao { protected final String host; protected final int port; protected final String key; protected final String username; protected final String password; - AbstractLogstashIndexerDao(String host, int port, String key, String username, String password) { + public AbstractLogstashIndexerDao(String host, int port, String key, String username, String password) { this.host = host; this.port = port; this.key = key; diff --git a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java index 88f33f0b..edb40760 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java @@ -25,7 +25,6 @@ package jenkins.plugins.logstash.persistence; import hudson.model.Action; -import hudson.model.Computer; import hudson.model.Environment; import hudson.model.Result; import hudson.model.AbstractBuild; @@ -187,11 +186,13 @@ public BuildData(AbstractBuild build, Date currentTime, TaskListener liste public BuildData(Run build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Executor executor = build.getExecutor(); - if (executor == null) { + Node node = build.getExecutor().getOwner().getNode(); + if (node == null) { buildHost = "master"; + buildLabel = "master"; } else { - buildHost = StringUtils.isBlank(executor.getDisplayName()) ? "master" : executor.getDisplayName(); + buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); + buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); } rootProjectName = projectName; diff --git a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java new file mode 100644 index 00000000..36827135 --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java @@ -0,0 +1,163 @@ +package jenkins.plugins.logstash; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.powermock.api.mockito.PowerMockito.when; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.Result; +import hudson.model.Slave; +import hudson.model.queue.QueueTaskFuture; +import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; +import jenkins.plugins.logstash.persistence.IndexerDaoFactory; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; +import net.sf.json.JSONArray; +import net.sf.json.JSONObject; + +@RunWith(PowerMockRunner.class) +@PowerMockIgnore({"javax.crypto.*"}) +@PrepareForTest(IndexerDaoFactory.class) +public class LogstashIntegrationTest +{ + @Rule + public JenkinsRule jenkins = new JenkinsRule(); + + private Slave slave; + + private FreeStyleProject project; + + private MemoryDao memoryDao; + + @Before + public void setup() throws Exception + { + PowerMockito.mockStatic(IndexerDaoFactory.class); + LogstashInstallation.Descriptor descriptor = LogstashInstallation.getLogstashDescriptor(); + descriptor.type = IndexerType.SYSLOG; + descriptor.host = "localhost"; + descriptor.port = 1; + descriptor.username = "username"; + descriptor.password = "password"; + descriptor.key = "key"; + + memoryDao = new MemoryDao(); + when(IndexerDaoFactory.getInstance(IndexerType.SYSLOG, descriptor.host, descriptor.port, descriptor.key, + descriptor.username, descriptor.password)).thenReturn(memoryDao); + slave = jenkins.createSlave(); + slave.setLabelString("myLabel"); + project = jenkins.createFreeStyleProject(); + + } + + @Test + public void test_buildWrapperOnMaster() throws Exception + { + project.getBuildWrappersList().add(new LogstashBuildWrapper()); + QueueTaskFuture f = project.scheduleBuild2(0); + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(3)); + JSONObject firstLine = dataLines.get(0); + JSONObject lastLine = dataLines.get(dataLines.size()-1); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo("Jenkins")); + assertThat(data.getString("buildLabel"),equalTo("master")); + assertThat(lastLine.getJSONArray("message").get(0).toString(),equalTo("Finished: SUCCESS")); + } + + @Test + public void test_buildWrapperOnSlave() throws Exception + { + project.getBuildWrappersList().add(new LogstashBuildWrapper()); + project.setAssignedNode(slave); + + QueueTaskFuture f = project.scheduleBuild2(0); + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(3)); + JSONObject firstLine = dataLines.get(0); + JSONObject lastLine = dataLines.get(dataLines.size()-1); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo(slave.getDisplayName())); + assertThat(data.getString("buildLabel"),equalTo(slave.getLabelString())); + assertThat(lastLine.getJSONArray("message").get(0).toString(),equalTo("Finished: SUCCESS")); + } + + @Test + public void test_buildNotifierOnMaster() throws Exception + { + project.getPublishersList().add(new LogstashNotifier(10, false)); + QueueTaskFuture f = project.scheduleBuild2(0); + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(1)); + JSONObject firstLine = dataLines.get(0); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo("Jenkins")); + assertThat(data.getString("buildLabel"),equalTo("master")); + } + + @Test + public void test_buildNotifierOnSlave() throws Exception + { + project.getPublishersList().add(new LogstashNotifier(10, false)); + project.setAssignedNode(slave); + QueueTaskFuture f = project.scheduleBuild2(0); + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(1)); + JSONObject firstLine = dataLines.get(0); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo(slave.getDisplayName())); + assertThat(data.getString("buildLabel"),equalTo(slave.getLabelString())); + } + + private static class MemoryDao extends AbstractLogstashIndexerDao + { + List output = new ArrayList<>(); + + public MemoryDao() + { + super("localhost", 1, "key", "username", "password"); + } + + @Override + public IndexerType getIndexerType() + { + // TODO Auto-generated method stub + return null; + } + + @Override + public void push(String data) throws IOException + { + JSONObject json = JSONObject.fromObject(data); + output.add(json); + } + + public List getOutput() + { + return output; + } + } +} diff --git a/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java b/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java index 27a30376..9769748a 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java @@ -17,7 +17,6 @@ import java.io.IOException; import java.io.OutputStream; import java.io.PrintStream; -import java.util.Arrays; import org.junit.After; import org.junit.Before; @@ -26,7 +25,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; @SuppressWarnings("rawtypes") @@ -76,18 +75,21 @@ public Result getResult() { @Mock LogstashWriter mockWriter; @Mock Launcher mockLauncher; @Mock BuildListener mockListener; + @Mock AbstractProject mockProject; ByteArrayOutputStream errorBuffer; PrintStream errorStream; LogstashNotifier notifier; MockRun mockRun; + @Before public void before() throws Exception { errorBuffer = new ByteArrayOutputStream(); errorStream = new PrintStream(errorBuffer, true); - mockRun = new MockRun(mock(AbstractProject.class)); + when(mockProject.assignBuildNumber()).thenReturn(1); + mockRun = new MockRun(mockProject); when(mockListener.getLogger()).thenReturn(errorStream); diff --git a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java index 3d191a83..735a6178 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java @@ -17,7 +17,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.*; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -97,9 +97,7 @@ public void before() throws Exception { when(mockBuild.getSensitiveBuildVariables()).thenReturn(Collections.emptySet()); when(mockBuild.getEnvironments()).thenReturn(null); when(mockBuild.getAction(AbstractTestResultAction.class)).thenReturn(mockTestResultAction); - when(mockBuild.getLog(0)).thenReturn(Arrays.asList()); when(mockBuild.getLog(3)).thenReturn(Arrays.asList("line 1", "line 2", "line 3", "Log truncated...")); - when(mockBuild.getLog(Integer.MAX_VALUE)).thenReturn(Arrays.asList("line 1", "line 2", "line 3", "line 4")); when(mockBuild.getEnvironment(null)).thenReturn(new EnvVars()); when(mockBuild.getExecutor()).thenReturn(mockExecutor); when(mockExecutor.getOwner()).thenReturn(mockComputer); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java index c623be66..2e445b21 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java @@ -23,7 +23,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; import hudson.EnvVars; @@ -73,7 +73,6 @@ public void before() throws Exception { when(mockBuild.getDisplayName()).thenReturn("BuildData Test"); when(mockBuild.getFullDisplayName()).thenReturn("BuildData Test #123456"); when(mockBuild.getDescription()).thenReturn("Mock project for testing BuildData"); - when(mockBuild.getProject()).thenReturn(mockProject); when(mockBuild.getParent()).thenReturn(mockProject); when(mockBuild.getNumber()).thenReturn(123456); when(mockBuild.getTimestamp()).thenReturn(new GregorianCalendar()); From c252ac815e0825b3fcbc60de7d03626261dd3e0c Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 27 Dec 2017 21:05:31 +0100 Subject: [PATCH 03/17] refactoring the plugin used a ToolInstallation for configuration. But logstash is not a tool that can be installed. Instead this should be a GlobalConfiguration. The indexers have different configuration requirements. Host and port are common, but things like username are not required everywhere. Also the previous implementation was totally unflexible. In order to add a new indexer one has to modify the sources. By configuring the indexers via an extension point we make it easily possible to have individual configuration jelly for each type. Adding a new indexer is just adding new classes. So it can be even moved to a separate plugin. Many of the fields in the plugin were public or package private. They have been made private or protected with corresponding getters and setters and direct calls to the fields replaced. Get the charset of a run and use it whenever we need to convert the String to a byte array Use DateFormatter from apache common lang to avoid potential threading problems. --- pom.xml | 11 +- .../logstash/LogstashBuildWrapper.java | 4 +- .../logstash/LogstashConfiguration.java | 160 ++++++++++++++++++ .../logstash/LogstashInstallation.java | 90 +++++----- .../plugins/logstash/LogstashNotifier.java | 4 +- .../logstash/LogstashOutputStream.java | 2 +- .../plugins/logstash/LogstashWriter.java | 46 +++-- .../logstash/configuration/ElasticSearch.java | 120 +++++++++++++ .../configuration/LogstashIndexer.java | 156 +++++++++++++++++ .../logstash/configuration/RabbitMq.java | 100 +++++++++++ .../plugins/logstash/configuration/Redis.java | 89 ++++++++++ .../logstash/configuration/Syslog.java | 75 ++++++++ .../AbstractLogstashIndexerDao.java | 40 +++-- .../logstash/persistence/BuildData.java | 118 ++++++++++--- .../persistence/ElasticSearchDao.java | 60 +++++-- .../persistence/IndexerDaoFactory.java | 115 ------------- .../persistence/LogstashIndexerDao.java | 15 +- .../logstash/persistence/RabbitMqDao.java | 43 +++-- .../logstash/persistence/RedisDao.java | 31 ++-- .../logstash/persistence/SyslogDao.java | 60 +++---- .../LogstashConfiguration/config.jelly | 8 + .../help-logstashIndexer.html | 1 + .../LogstashInstallation/global.jelly | 32 +--- .../LogstashInstallation/help-key.html | 5 - .../help-syslogProtocol.html | 3 - .../LogstashInstallation/help-type.html | 3 - .../plugins/logstash/Messages.properties | 2 +- .../ElasticSearch/configure-advanced.jelly | 12 ++ .../ElasticSearch}/help-host.html | 2 +- .../configuration/ElasticSearch/help-key.html | 3 + .../ElasticSearch}/help-password.html | 0 .../ElasticSearch}/help-username.html | 0 .../LogstashIndexer/config.jelly | 10 ++ .../LogstashIndexer/help-host.html | 3 + .../LogstashIndexer}/help-port.html | 0 .../RabbitMq/configure-advanced.jelly | 12 ++ .../configuration/RabbitMq/help-password.html | 4 + .../configuration/RabbitMq/help-queue.html | 3 + .../configuration/RabbitMq/help-username.html | 4 + .../Redis/configure-advanced.jelly | 9 + .../configuration/Redis/help-key.html | 3 + .../configuration/Redis/help-password.html | 4 + .../Syslog/configure-advanced.jelly | 9 + .../Syslog/help-messageFormat.html} | 2 +- .../Syslog/help-syslogProtocol.html | 3 + .../logstash/LogstashBuildWrapperTest.java | 2 +- .../LogstashConfigurationMigrationTest.java | 133 +++++++++++++++ .../logstash/LogstashConfigurationTest.java | 59 +++++++ .../LogstashConfigurationTestBase.java | 26 +++ .../logstash/LogstashIntegrationTest.java | 60 ++----- .../logstash/LogstashOutputStreamTest.java | 6 +- .../plugins/logstash/LogstashWriterTest.java | 28 +-- .../configuration/ElasticSearchTest.java | 62 +++++++ .../configuration/LogstashIndexerTest.java | 54 ++++++ .../logstash/configuration/RabbitMqTest.java | 62 +++++++ .../logstash/configuration/RedisTest.java | 56 ++++++ .../logstash/configuration/SyslogTest.java | 43 +++++ .../AbstractLogstashIndexerDaoTest.java | 13 +- .../logstash/persistence/BuildDataTest.java | 12 +- .../persistence/ElasticSearchDaoTest.java | 44 ++--- .../persistence/IndexerDaoFactoryTest.java | 43 ----- .../logstash/persistence/MemoryDao.java | 30 ++++ .../logstash/persistence/RabbitMqDaoTest.java | 15 +- .../logstash/persistence/RedisDaoTest.java | 11 +- .../logstash/persistence/SyslogDaoTest.java | 14 +- .../logstash/persistence/SyslogDaoTestIT.java | 40 ++--- src/test/resources/elasticSearch.xml | 11 ++ src/test/resources/rabbitmq.xml | 10 ++ src/test/resources/redis.xml | 10 ++ src/test/resources/syslog.xml | 10 ++ 70 files changed, 1810 insertions(+), 520 deletions(-) create mode 100644 src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/Redis.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/Syslog.java delete mode 100644 src/main/java/jenkins/plugins/logstash/persistence/IndexerDaoFactory.java create mode 100644 src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/config.jelly create mode 100644 src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/help-logstashIndexer.html delete mode 100644 src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-key.html delete mode 100644 src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogProtocol.html delete mode 100644 src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-type.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly rename src/main/resources/jenkins/plugins/logstash/{LogstashInstallation => configuration/ElasticSearch}/help-host.html (54%) create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html rename src/main/resources/jenkins/plugins/logstash/{LogstashInstallation => configuration/ElasticSearch}/help-password.html (100%) rename src/main/resources/jenkins/plugins/logstash/{LogstashInstallation => configuration/ElasticSearch}/help-username.html (100%) create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html rename src/main/resources/jenkins/plugins/logstash/{LogstashInstallation => configuration/LogstashIndexer}/help-port.html (100%) create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/configure-advanced.jelly create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-password.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-queue.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-username.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/Redis/configure-advanced.jelly create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-key.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-password.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/Syslog/configure-advanced.jelly rename src/main/resources/jenkins/plugins/logstash/{LogstashInstallation/help-syslogFormat.html => configuration/Syslog/help-messageFormat.html} (68%) create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-syslogProtocol.html create mode 100644 src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/LogstashConfigurationTestBase.java create mode 100644 src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java delete mode 100644 src/test/java/jenkins/plugins/logstash/persistence/IndexerDaoFactoryTest.java create mode 100644 src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java create mode 100644 src/test/resources/elasticSearch.xml create mode 100644 src/test/resources/rabbitmq.xml create mode 100644 src/test/resources/redis.xml create mode 100644 src/test/resources/syslog.xml diff --git a/pom.xml b/pom.xml index e9da92c5..abaac5d3 100644 --- a/pom.xml +++ b/pom.xml @@ -5,12 +5,14 @@ UTF-8 true + 2.60.3 + 2.23 org.jenkins-ci.plugins plugin - 1.608 + 2.15 @@ -86,7 +88,7 @@ org.mockito mockito-core - 2.10.0 + 2.13.0 test @@ -136,6 +138,11 @@ 2.0.0-beta.5 test + + org.jenkins-ci.plugins.workflow + workflow-step-api + 1.15 + diff --git a/src/main/java/jenkins/plugins/logstash/LogstashBuildWrapper.java b/src/main/java/jenkins/plugins/logstash/LogstashBuildWrapper.java index 2e4a0100..8587696e 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashBuildWrapper.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashBuildWrapper.java @@ -46,7 +46,6 @@ /** * Build wrapper that decorates the build's logger to insert a - * {@link LogstashNote} on each output line. * * @author K Jonathan Harker */ @@ -102,13 +101,14 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) { return los; } + @Override public DescriptorImpl getDescriptor() { return (DescriptorImpl) super.getDescriptor(); } // Method to encapsulate calls for unit-testing LogstashWriter getLogStashWriter(AbstractBuild build, OutputStream errorStream) { - return new LogstashWriter(build, errorStream, null); + return new LogstashWriter(build, errorStream, null, build.getCharset()); } /** diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java new file mode 100644 index 00000000..6cb415cc --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -0,0 +1,160 @@ +package jenkins.plugins.logstash; + +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; + +import javax.annotation.CheckForNull; + +import org.kohsuke.stapler.StaplerRequest; + +import com.cloudbees.syslog.MessageFormat; +import com.cloudbees.syslog.integration.jul.SyslogMessageFormatter; + +import hudson.DescriptorExtensionList; +import hudson.Extension; +import hudson.init.InitMilestone; +import hudson.init.Initializer; +import jenkins.model.GlobalConfiguration; +import jenkins.plugins.logstash.LogstashInstallation.Descriptor; +import jenkins.plugins.logstash.configuration.ElasticSearch; +import jenkins.plugins.logstash.configuration.LogstashIndexer; +import jenkins.plugins.logstash.configuration.LogstashIndexer.LogstashIndexerDescriptor; +import jenkins.plugins.logstash.configuration.RabbitMq; +import jenkins.plugins.logstash.configuration.Redis; +import jenkins.plugins.logstash.configuration.Syslog; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.SyslogFormat; +import net.sf.json.JSONObject; + +@Extension +public class LogstashConfiguration extends GlobalConfiguration +{ + private static final Logger LOGGER = Logger.getLogger(LogstashConfiguration.class.getName()); + private LogstashIndexer logstashIndexer; + private boolean dataMigrated = false; + + public LogstashConfiguration() + { + load(); + } + + public LogstashIndexer getLogstashIndexer() + { + return logstashIndexer; + } + + public void setLogstashIndexer(LogstashIndexer logstashIndexer) + { + this.logstashIndexer = logstashIndexer; + } + + @CheckForNull + public LogstashIndexerDao getIndexerInstance() + { + if (logstashIndexer != null) + { + return logstashIndexer.getInstance(); + } + return null; + } + + public List getIndexerTypes() + { + return LogstashIndexer.all(); + } + + @Initializer(after = InitMilestone.JOB_LOADED) + public void migrateData() + { + if (!dataMigrated) + { + Descriptor descriptor = LogstashInstallation.getLogstashDescriptor(); + if (descriptor.getType() != null) + { + IndexerType type = descriptor.getType(); + switch (type) + { + case REDIS: + LOGGER.log(Level.INFO, "Migrating logstash configuration for Redis"); + Redis redis = new Redis(); + redis.setHost(descriptor.getHost()); + redis.setPort(descriptor.getPort()); + redis.setKey(descriptor.getKey()); + redis.setPassword(descriptor.getPassword()); + logstashIndexer = redis; + break; + case ELASTICSEARCH: + LOGGER.log(Level.INFO, "Migrating logstash configuration for Elastic Search"); + ElasticSearch es = new ElasticSearch(); + es.setHost(descriptor.getHost()); + es.setPort(descriptor.getPort()); + es.setKey(descriptor.getKey()); + es.setUsername(descriptor.getUsername()); + es.setPassword(descriptor.getPassword()); + logstashIndexer = es; + break; + case RABBIT_MQ: + LOGGER.log(Level.INFO, "Migrating logstash configuration for RabbitMQ"); + RabbitMq rabbitMq = new RabbitMq(); + rabbitMq.setHost(descriptor.getHost()); + rabbitMq.setPort(descriptor.getPort()); + rabbitMq.setQueue(descriptor.getKey()); + rabbitMq.setUsername(descriptor.getUsername()); + rabbitMq.setPassword(descriptor.getPassword()); + logstashIndexer = rabbitMq; + break; + case SYSLOG: + LOGGER.log(Level.INFO, "Migrating logstash configuration for SYSLOG"); + Syslog syslog = new Syslog(); + syslog.setHost(descriptor.getHost()); + syslog.setPort(descriptor.getPort()); + syslog.setSyslogProtocol(descriptor.getSyslogProtocol()); + switch (descriptor.getSyslogFormat()) + { + case RFC3164: + syslog.setMessageFormat(MessageFormat.RFC_3164); + break; + case RFC5424: + syslog.setMessageFormat(MessageFormat.RFC_5424); + break; + default: + syslog.setMessageFormat(MessageFormat.RFC_3164); + break; + } + logstashIndexer = syslog; + break; + default: + LOGGER.log(Level.INFO, "unknown logstash Indexer type: " + type); + break; + } + } + dataMigrated = true; + save(); + } + } + + @Override + public boolean configure(StaplerRequest staplerRequest, JSONObject json) throws FormException + { + JSONObject j = json.getJSONObject("logstashIndexer"); + String clazz = j.getString("stapler-class"); + if (logstashIndexer == null || !logstashIndexer.getClass().getName().equals(clazz)) + { + staplerRequest.bindJSON(this, json); + } + else + { + logstashIndexer.reconfigure(staplerRequest, j); + } + save(); + return true; + } + + public static LogstashConfiguration getInstance() + { + return GlobalConfiguration.all().get(LogstashConfiguration.class); + } + +} diff --git a/src/main/java/jenkins/plugins/logstash/LogstashInstallation.java b/src/main/java/jenkins/plugins/logstash/LogstashInstallation.java index a806d95b..48e6e701 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashInstallation.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashInstallation.java @@ -63,66 +63,72 @@ public static Descriptor getLogstashDescriptor() { @Extension public static final class Descriptor extends ToolDescriptor { - public IndexerType type; - public SyslogFormat syslogFormat; - public SyslogProtocol syslogProtocol; - public String host; - public Integer port = -1; - public String username; - public String password; - public String key; + private transient IndexerType type; + private transient SyslogFormat syslogFormat; + private transient SyslogProtocol syslogProtocol; + private transient String host; + private transient Integer port = -1; + private transient String username; + private transient String password; + private transient String key; public Descriptor() { super(); load(); } - @Override - public boolean configure(StaplerRequest req, JSONObject formData) throws FormException { - req.bindJSON(this, formData.getJSONObject("logstash")); - save(); - return super.configure(req, formData); + + public IndexerType getType() + { + return type; } - @Override - public ToolInstallation newInstance(StaplerRequest req, JSONObject formData) throws FormException { - req.bindJSON(this, formData.getJSONObject("logstash")); - save(); - return super.newInstance(req, formData); + + public SyslogFormat getSyslogFormat() + { + return syslogFormat; } - @Override - public String getDisplayName() { - return Messages.DisplayName(); + + public SyslogProtocol getSyslogProtocol() + { + return syslogProtocol; } - /* - * Form validation methods - */ - public FormValidation doCheckInteger(@QueryParameter("value") String value) { - try { - Integer.parseInt(value); - } catch (NumberFormatException e) { - return FormValidation.error(Messages.ValueIsInt()); - } - - return FormValidation.ok(); + + public String getHost() + { + return host; } - public FormValidation doCheckHost(@QueryParameter("value") String value) { - if (StringUtils.isBlank(value)) { - return FormValidation.warning(Messages.PleaseProvideHost()); - } - return FormValidation.ok(); + public Integer getPort() + { + return port; } - public FormValidation doCheckString(@QueryParameter("value") String value) { - if (StringUtils.isBlank(value)) { - return FormValidation.error(Messages.ValueIsRequired()); - } - return FormValidation.ok(); + public String getUsername() + { + return username; + } + + + public String getPassword() + { + return password; + } + + + public String getKey() + { + return key; + } + + + @Override + public String getDisplayName() { + return Messages.DisplayName(); } } } diff --git a/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java b/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java index 722ecc93..1dea1a64 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java @@ -85,9 +85,10 @@ private boolean perform(Run run, TaskListener listener) { // Method to encapsulate calls for unit-testing LogstashWriter getLogStashWriter(Run run, OutputStream errorStream, TaskListener listener) { - return new LogstashWriter(run, errorStream, listener); + return new LogstashWriter(run, errorStream, listener, run.getCharset()); } + @Override public BuildStepMonitor getRequiredMonitorService() { // We don't call Run#getPreviousBuild() so no external synchronization between builds is required return BuildStepMonitor.NONE; @@ -106,6 +107,7 @@ public boolean isApplicable(@SuppressWarnings("rawtypes") Class build; - final TaskListener listener; - final BuildData buildData; - final String jenkinsUrl; - final LogstashIndexerDao dao; + private final OutputStream errorStream; + private final Run build; + private final TaskListener listener; + private final BuildData buildData; + private final String jenkinsUrl; + private final LogstashIndexerDao dao; private boolean connectionBroken; + private Charset charset; - public LogstashWriter(Run run, OutputStream error, TaskListener listener) { + public LogstashWriter(Run run, OutputStream error, TaskListener listener, Charset charset) { this.errorStream = error != null ? error : System.err; this.build = run; this.listener = listener; + this.charset = charset; this.dao = this.getDaoOrNull(); if (this.dao == null) { this.jenkinsUrl = ""; @@ -128,9 +130,13 @@ public boolean isConnectionBroken() { } // Method to encapsulate calls for unit-testing - LogstashIndexerDao getDao() throws InstantiationException { - LogstashInstallation.Descriptor descriptor = LogstashInstallation.getLogstashDescriptor(); - return IndexerDaoFactory.getInstance(descriptor.type, descriptor.host, descriptor.port, descriptor.key, descriptor.username, descriptor.password); + LogstashIndexerDao getIndexerDao() { + return LogstashConfiguration.getInstance().getIndexerInstance(); + } + + LogstashIndexerDao getDao() + { + return dao; } BuildData getBuildData() { @@ -153,7 +159,7 @@ private void write(List lines) { try { dao.push(payload.toString()); } catch (IOException e) { - String msg = "[logstash-plugin]: Failed to send log data to " + dao.getIndexerType() + ":" + dao.getDescription() + ".\n" + + String msg = "[logstash-plugin]: Failed to send log data: " + dao.getDescription() + ".\n" + "[logstash-plugin]: No Further logs will be sent to " + dao.getDescription() + ".\n" + ExceptionUtils.getStackTrace(e); logErrorMessage(msg); @@ -168,8 +174,13 @@ private void write(List lines) { */ private LogstashIndexerDao getDaoOrNull() { try { - return getDao(); - } catch (InstantiationException e) { + LogstashIndexerDao dao = getIndexerDao(); + if (dao == null) + { + logErrorMessage("[logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.\n"); + } + return dao; + } catch (IllegalArgumentException e) { String msg = ExceptionUtils.getMessage(e) + "\n" + "[logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.\n"; @@ -184,11 +195,16 @@ private LogstashIndexerDao getDaoOrNull() { private void logErrorMessage(String msg) { try { connectionBroken = true; - errorStream.write(msg.getBytes()); + errorStream.write(msg.getBytes(charset)); errorStream.flush(); } catch (IOException ex) { // This should never happen, but if it does we just have to let it go. ex.printStackTrace(); } } + + public Charset getCharset() + { + return charset; + } } diff --git a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java new file mode 100644 index 00000000..8045ac24 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java @@ -0,0 +1,120 @@ +package jenkins.plugins.logstash.configuration; + +import java.net.MalformedURLException; +import java.net.URL; + +import org.apache.commons.lang.StringUtils; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; + +import hudson.Extension; +import hudson.util.FormValidation; +import hudson.util.Secret; +import jenkins.plugins.logstash.Messages; +import jenkins.plugins.logstash.persistence.ElasticSearchDao; + +public class ElasticSearch extends LogstashIndexer +{ + protected String key; + protected String username; + protected Secret password; + + @DataBoundConstructor + public ElasticSearch() + { + } + + public String getKey() + { + return key; + } + + @DataBoundSetter + public void setKey(String key) + { + this.key = key; + } + + public String getUsername() + { + return username; + } + + @DataBoundSetter + public void setUsername(String username) + { + this.username = username; + } + + public String getPassword() + { + return Secret.toString(password); + } + + @DataBoundSetter + public void setPassword(String password) + { + this.password = Secret.fromString(password); + } + + @Override + protected boolean shouldRefreshInstance() + { + return super.shouldRefreshInstance() || + !instance.getPassword().equals(Secret.toString(password)) || + !StringUtils.equals(instance.getUsername(), username) || + !StringUtils.equals(instance.getKey(), key); + } + + @Override + public ElasticSearchDao createIndexerInstance() + { + return new ElasticSearchDao(host, port, key, username, Secret.toString(password)); + } + + @Extension + public static class ElasticSearchDescriptor extends LogstashIndexerDescriptor + { + @Override + public String getDisplayName() + { + return "Elastic Search"; + } + + @Override + public int getDefaultPort() + { + return 9300; + } + + @Override + public FormValidation doCheckHost(@QueryParameter("value") String value) + { + if (StringUtils.isBlank(value)) + { + return FormValidation.warning(Messages.PleaseProvideHost()); + } + try + { + new URL(value); + } + catch (MalformedURLException e) + { + return FormValidation.error(e.getMessage()); + } + return FormValidation.ok(); + } + + public FormValidation doCheckKey(@QueryParameter("value") String value) + { + if (StringUtils.isBlank(value)) + { + return FormValidation.error(Messages.ValueIsRequired()); + } + + return FormValidation.ok(); + } + + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java new file mode 100644 index 00000000..1ca17e9a --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -0,0 +1,156 @@ +package jenkins.plugins.logstash.configuration; + +import javax.annotation.Nonnull; + +import org.apache.commons.lang.StringUtils; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.StaplerRequest; + +import hudson.DescriptorExtensionList; +import hudson.ExtensionPoint; +import hudson.model.AbstractDescribableImpl; +import hudson.model.Descriptor; +import hudson.model.ReconfigurableDescribable; +import hudson.model.Descriptor.FormException; +import hudson.util.FormValidation; +import jenkins.model.Jenkins; +import jenkins.plugins.logstash.Messages; +import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; +import net.sf.json.JSONObject; + +public abstract class LogstashIndexer + extends AbstractDescribableImpl> + implements ExtensionPoint, ReconfigurableDescribable> +{ + protected String host; + protected int port; + + protected transient T instance; + + /** + * Returns the host for connecting to the indexer. + * + * @return Host of the indexer + */ + public String getHost() + { + return host; + } + + /** + * Sets the host for connecting to the indexer. + * + * @param host + * host to connect to. + */ + @DataBoundSetter + public void setHost(String host) + { + this.host = host; + } + + /** + * Returns the port for connecting to the indexer. + * + * @return Port of the indexer + */ + public int getPort() + { + return port; + } + + /** + * Sets the port used for connecting to the indexer + * + * @param port + * The port of the indexer + */ + @DataBoundSetter + public void setPort(int port) + { + this.port = port; + } + + /** + * Gets the instance of the actual {@link AbstractLogstashIndexerDao} that is represented by this + * configuration. + * When changing the configuration a new instance will be created. + * + * @return {@link AbstractLogstashIndexerDao} instance + */ + @Nonnull + public final T getInstance() + { + if (shouldRefreshInstance()) + { + instance = createIndexerInstance(); + } + return instance; + } + + + /** + * Creates a new {@link AbstractLogstashIndexerDao} instance corresponding to this configuration. + * + * @return {@link AbstractLogstashIndexerDao} instance + */ + protected abstract T createIndexerInstance(); + + protected boolean shouldRefreshInstance() + { + if (instance == null) + { + return true; + } + + boolean matches = StringUtils.equals(instance.getHost(), host) && + (instance.getPort() == port); + return !matches; + } + + @SuppressWarnings("unchecked") + public static DescriptorExtensionList, Descriptor>> all() + { + return Jenkins.getInstance().getDescriptorList(LogstashIndexer.class); + } + + public static abstract class LogstashIndexerDescriptor extends Descriptor> + { + /* + * Form validation methods + */ + public FormValidation doCheckPort(@QueryParameter("value") String value) + { + try + { + Integer.parseInt(value); + } + catch (NumberFormatException e) + { + return FormValidation.error(Messages.ValueIsInt()); + } + + return FormValidation.ok(); + } + + public FormValidation doCheckHost(@QueryParameter("value") String value) + { + if (StringUtils.isBlank(value)) + { + return FormValidation.warning(Messages.PleaseProvideHost()); + } + + return FormValidation.ok(); + } + + public abstract int getDefaultPort(); + } + + @Override + public LogstashIndexer reconfigure(StaplerRequest req, JSONObject form) throws FormException + { + req.bindJSON(this, form); + return this; + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java new file mode 100644 index 00000000..3dce6418 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java @@ -0,0 +1,100 @@ +package jenkins.plugins.logstash.configuration; + +import org.apache.commons.lang.StringUtils; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; + +import hudson.Extension; +import hudson.util.FormValidation; +import hudson.util.Secret; +import jenkins.plugins.logstash.Messages; +import jenkins.plugins.logstash.persistence.RabbitMqDao; + +public class RabbitMq extends LogstashIndexer +{ + + protected String queue; + protected String username; + protected Secret password; + + @DataBoundConstructor + public RabbitMq() + { + } + + public String getQueue() + { + return queue; + } + + @DataBoundSetter + public void setQueue(String queue) + { + this.queue = queue; + } + + public String getUsername() + { + return username; + } + + @DataBoundSetter + public void setUsername(String username) + { + this.username = username; + } + + public String getPassword() + { + return Secret.toString(password); + } + + @DataBoundSetter + public void setPassword(String password) + { + this.password = Secret.fromString(password); + } + + @Override + protected boolean shouldRefreshInstance() + { + return super.shouldRefreshInstance() || + !instance.getPassword().equals(Secret.toString(password)) || + !StringUtils.equals(instance.getUsername(), username) || + !StringUtils.equals(instance.getQueue(), queue); + } + + @Override + public RabbitMqDao createIndexerInstance() + { + return new RabbitMqDao(host, port, queue, username, Secret.toString(password)); + } + + @Extension + public static class RabbitMqDescriptor extends LogstashIndexerDescriptor + { + @Override + public String getDisplayName() + { + return "RabbitMQ"; + } + + @Override + public int getDefaultPort() + { + return 5672; + } + + public FormValidation doCheckQueue(@QueryParameter("value") String value) + { + if (StringUtils.isBlank(value)) + { + return FormValidation.error(Messages.ValueIsRequired()); + } + + return FormValidation.ok(); + } + + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Redis.java b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java new file mode 100644 index 00000000..2f3cd4f7 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java @@ -0,0 +1,89 @@ +package jenkins.plugins.logstash.configuration; + +import org.apache.commons.lang.StringUtils; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; + +import hudson.Extension; +import hudson.util.FormValidation; +import hudson.util.Secret; +import jenkins.plugins.logstash.Messages; +import jenkins.plugins.logstash.persistence.RedisDao; + +public class Redis extends LogstashIndexer +{ + + protected String key; + protected Secret password; + + @DataBoundConstructor + public Redis() + { + } + + public String getKey() + { + return key; + } + + @DataBoundSetter + public void setKey(String key) + { + this.key = key; + } + + public String getPassword() + { + return Secret.toString(password); + } + + @DataBoundSetter + public void setPassword(String password) + { + this.password = Secret.fromString(password); + } + + @Override + protected boolean shouldRefreshInstance() + { + return super.shouldRefreshInstance() || + !instance.getPassword().equals(Secret.toString(password))|| + !StringUtils.equals(instance.getKey(), key); + } + + + @Override + public RedisDao createIndexerInstance() + { + return new RedisDao(host, port, key, Secret.toString(password)); + } + + @Extension + public static class RedisDescriptor extends LogstashIndexerDescriptor + { + + @Override + public String getDisplayName() + { + return "Redis"; + } + + @Override + public int getDefaultPort() + { + return 6379; + } + + public FormValidation doCheckKey(@QueryParameter("value") String value) + { + if (StringUtils.isBlank(value)) + { + return FormValidation.error(Messages.ValueIsRequired()); + } + + return FormValidation.ok(); + } + + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java new file mode 100644 index 00000000..f69fdf3f --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java @@ -0,0 +1,75 @@ +package jenkins.plugins.logstash.configuration; + +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; + +import com.cloudbees.syslog.MessageFormat; + +import hudson.Extension; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.SyslogProtocol; +import jenkins.plugins.logstash.persistence.SyslogDao; + +public class Syslog extends LogstashIndexer +{ + private MessageFormat messageFormat; + private SyslogProtocol syslogProtocol; + + @DataBoundConstructor + public Syslog() + { + } + + public MessageFormat getMessageFormat() + { + return messageFormat; + } + + @DataBoundSetter + public void setMessageFormat(MessageFormat messageFormat) + { + this.messageFormat = messageFormat; + } + + public SyslogProtocol getSyslogProtocol() + { + return syslogProtocol; + } + + @DataBoundSetter() + public void setSyslogProtocol(SyslogProtocol syslogProtocol) + { + this.syslogProtocol = syslogProtocol; + } + + @Override + protected boolean shouldRefreshInstance() + { + return super.shouldRefreshInstance() || + !instance.getMessageFormat().equals(messageFormat); + } + + @Override + public SyslogDao createIndexerInstance() + { + SyslogDao syslogDao = new SyslogDao(host, port); + syslogDao.setMessageFormat(messageFormat); + return syslogDao; + } + + @Extension + public static class SyslogDescriptor extends LogstashIndexerDescriptor + { + + @Override + public String getDisplayName() + { + return "Syslog"; + } + + @Override + public int getDefaultPort() + { + return 519; + } + } +} diff --git a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java index c17b61ce..0baeb7c4 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java @@ -24,6 +24,7 @@ package jenkins.plugins.logstash.persistence; +import java.nio.charset.Charset; import java.util.Calendar; import java.util.List; @@ -38,20 +39,16 @@ * @since 1.0.0 */ public abstract class AbstractLogstashIndexerDao implements LogstashIndexerDao { - protected final String host; - protected final int port; - protected final String key; - protected final String username; - protected final String password; + private final String host; + private final int port; + private Charset charset; - public AbstractLogstashIndexerDao(String host, int port, String key, String username, String password) { + public AbstractLogstashIndexerDao(String host, int port) + { this.host = host; this.port = port; - this.key = key; - this.username = username; - this.password = password; - - if (StringUtils.isBlank(host)) { + if (StringUtils.isBlank(host)) + { throw new IllegalArgumentException("host name is required"); } } @@ -70,6 +67,27 @@ public JSONObject buildPayload(BuildData buildData, String jenkinsUrl, List failedTestsWithErrorDetail; - List failedTests; + private int totalCount, skipCount, failCount, passCount; + private List failedTestsWithErrorDetail; + private List failedTests; public static class FailedTest { - final String fullName, errorDetails; + private final String fullName, errorDetails; - public FailedTest(String fullName, String errorDetails) { - super(); - this.fullName = fullName; - this.errorDetails = errorDetails; - } + public FailedTest(String fullName, String errorDetails) { + super(); + this.fullName = fullName; + this.errorDetails = errorDetails; + } + + public String getFullName() + { + return fullName; + } + + public String getErrorDetails() + { + return errorDetails; + } } public TestData() { @@ -111,6 +122,36 @@ public TestData(Action action) { failedTestsWithErrorDetail.add(new FailedTest(result.getFullName(),result.getErrorDetails())); } } + + public int getTotalCount() + { + return totalCount; + } + + public int getSkipCount() + { + return skipCount; + } + + public int getFailCount() + { + return failCount; + } + + public int getPassCount() + { + return passCount; + } + + public List getFailedTestsWithErrorDetail() + { + return failedTestsWithErrorDetail; + } + + public List getFailedTests() + { + return failedTests; + } } protected String id; @@ -138,13 +179,22 @@ public TestData(Action action) { public BuildData(AbstractBuild build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Node node = build.getExecutor().getOwner().getNode(); - if (node == null) { - buildHost = "master"; - buildLabel = "master"; - } else { - buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); - buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); + Executor executor = build.getExecutor(); + if (executor == null) + { + buildHost = "master"; + buildLabel = "master"; + } + else + { + Node node = executor.getOwner().getNode(); + if (node == null) { + buildHost = "master"; + buildLabel = "master"; + } else { + buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); + buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); + } } // build.getDuration() is always 0 in Notifiers @@ -186,13 +236,22 @@ public BuildData(AbstractBuild build, Date currentTime, TaskListener liste public BuildData(Run build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Node node = build.getExecutor().getOwner().getNode(); - if (node == null) { - buildHost = "master"; - buildLabel = "master"; - } else { - buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); - buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); + Executor executor = build.getExecutor(); + if (executor == null) + { + buildHost = "master"; + buildLabel = "master"; + } + else + { + Node node = executor.getOwner().getNode(); + if (node == null) { + buildHost = "master"; + buildLabel = "master"; + } else { + buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); + buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); + } } rootProjectName = projectName; @@ -210,7 +269,16 @@ public BuildData(Run build, Date currentTime, TaskListener listener) { } private void initData(Run build, Date currentTime) { - result = build.getResult() == null ? null : build.getResult().toString(); + Result result = build.getResult(); + if (build.getResult() == null) + { + this.result = null; + } + else + { + this.result = result.toString(); + } + //result = build.getResult() == null ? null : build.getResult().toString(); id = build.getId(); projectName = build.getParent().getName(); fullProjectName = build.getParent().getFullName(); diff --git a/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java b/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java index 91c29df2..9962a01f 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java @@ -40,8 +40,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; +import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.Charset; import com.google.common.collect.Range; @@ -52,10 +54,14 @@ * @since 1.0.4 */ public class ElasticSearchDao extends AbstractLogstashIndexerDao { - final HttpClientBuilder clientBuilder; - final URI uri; - final String auth; - final Range successCodes = closedOpen(200,300); + private final HttpClientBuilder clientBuilder; + private final URI uri; + private final String auth; + private final Range successCodes = closedOpen(200,300); + + private String key; + private String username; + private String password; //primary constructor used by indexer factory public ElasticSearchDao(String host, int port, String key, String username, String password) { @@ -64,7 +70,11 @@ public ElasticSearchDao(String host, int port, String key, String username, Stri // Factored for unit testing ElasticSearchDao(HttpClientBuilder factory, String host, int port, String key, String username, String password) { - super(host, port, key, username, password); + super(host, port); + + this.key = key; + this.username = username; + this.password = password; if (StringUtils.isBlank(key)) { throw new IllegalArgumentException("elastic index name is required"); @@ -85,7 +95,7 @@ public ElasticSearchDao(String host, int port, String key, String username, Stri } if (StringUtils.isNotBlank(username)) { - auth = Base64.encodeBase64String((username + ":" + StringUtils.defaultString(password)).getBytes()); + auth = Base64.encodeBase64String((username + ":" + StringUtils.defaultString(password)).getBytes(Charset.defaultCharset())); } else { auth = null; } @@ -93,7 +103,32 @@ public ElasticSearchDao(String host, int port, String key, String username, Stri clientBuilder = factory == null ? HttpClientBuilder.create() : factory; } - HttpPost getHttpPost(String data) { + public URI getUri() + { + return uri; + } + + public String getKey() + { + return key; + } + + public String getUsername() + { + return username; + } + + public String getPassword() + { + return password; + } + + String getAuth() + { + return auth; + } + + protected HttpPost getHttpPost(String data) { HttpPost postRequest; postRequest = new HttpPost(uri); StringEntity input = new StringEntity(data, ContentType.APPLICATION_JSON); @@ -132,7 +167,7 @@ private String getErrorMessage(CloseableHttpResponse response) { PrintStream stream = null; try { byteStream = new ByteArrayOutputStream(); - stream = new PrintStream(byteStream); + stream = new PrintStream(byteStream, true, "UTF-8"); try { stream.print("HTTP error code: "); @@ -145,14 +180,15 @@ private String getErrorMessage(CloseableHttpResponse response) { stream.println(ExceptionUtils.getStackTrace(e)); } stream.flush(); - return byteStream.toString(); + return byteStream.toString("UTF-8"); + } + catch (UnsupportedEncodingException e) + { + return ExceptionUtils.getStackTrace(e); } finally { if (stream != null) { stream.close(); } } } - - @Override - public IndexerType getIndexerType() { return IndexerType.ELASTICSEARCH; } } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/IndexerDaoFactory.java b/src/main/java/jenkins/plugins/logstash/persistence/IndexerDaoFactory.java deleted file mode 100644 index ffd79345..00000000 --- a/src/main/java/jenkins/plugins/logstash/persistence/IndexerDaoFactory.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * The MIT License - * - * Copyright 2014 Rusty Gerard - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.plugins.logstash.persistence; - -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; - -import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; - -import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.exception.ExceptionUtils; - -/** - * Factory for AbstractLogstashIndexerDao objects. - * - * @author Rusty Gerard - * @since 1.0.0 - */ -public final class IndexerDaoFactory { - private static AbstractLogstashIndexerDao instance = null; - - private static final Map> INDEXER_MAP; - static { - Map> indexerMap = new HashMap>(); - - indexerMap.put(IndexerType.REDIS, RedisDao.class); - indexerMap.put(IndexerType.RABBIT_MQ, RabbitMqDao.class); - indexerMap.put(IndexerType.ELASTICSEARCH, ElasticSearchDao.class); - indexerMap.put(IndexerType.SYSLOG, SyslogDao.class); - - INDEXER_MAP = Collections.unmodifiableMap(indexerMap); - } - - /** - * Singleton instance accessor. - * - * @param type - * The type of indexer, not null - * @param host - * The host name or IP address of the indexer, not null - * @param port - * The port the indexer listens on - * @param key - * The subcollection to write to in the indexer, not null - * @param username - * The user name to authenticate with the indexer, nullable - * @param password - * The password to authenticate with the indexer, nullable - * @return The instance of the appropriate indexer DAO, never null - * @throws InstantiationException - */ - public static synchronized LogstashIndexerDao getInstance(IndexerType type, String host, Integer port, String key, String username, String password) throws InstantiationException { - if (!INDEXER_MAP.containsKey(type)) { - throw new InstantiationException("[logstash-plugin]: Unknown IndexerType '" + type + "'. Did you forget to configure the plugin?"); - } - - // Prevent NPE - port = (port == null ? -1 : port.intValue()); - - if (shouldRefreshInstance(type, host, port, key, username, password)) { - try { - Class indexerClass = INDEXER_MAP.get(type); - Constructor constructor = indexerClass.getConstructor(String.class, int.class, String.class, String.class, String.class); - instance = (AbstractLogstashIndexerDao) constructor.newInstance(host, port, key, username, password); - } catch (NoSuchMethodException e) { - throw new InstantiationException(ExceptionUtils.getRootCauseMessage(e)); - } catch (InvocationTargetException e) { - throw new InstantiationException(ExceptionUtils.getRootCauseMessage(e)); - } catch (IllegalAccessException e) { - throw new InstantiationException(ExceptionUtils.getRootCauseMessage(e)); - } - } - - return instance; - } - - private static boolean shouldRefreshInstance(IndexerType type, String host, int port, String key, String username, String password) { - if (instance == null) { - return true; - } - - boolean matches = (instance.getIndexerType() == type) && - StringUtils.equals(instance.host, host) && - (instance.port == port) && - StringUtils.equals(instance.key, key) && - StringUtils.equals(instance.username, username) && - StringUtils.equals(instance.password, password); - return !matches; - } -} diff --git a/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java index ea25d73b..8491dd8a 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java @@ -25,6 +25,7 @@ package jenkins.plugins.logstash.persistence; import java.io.IOException; +import java.nio.charset.Charset; import java.util.List; import net.sf.json.JSONObject; @@ -36,13 +37,13 @@ * @since 1.0.0 */ public interface LogstashIndexerDao { - static enum IndexerType { + public static enum IndexerType { REDIS, RABBIT_MQ, ELASTICSEARCH, SYSLOG } - + static enum SyslogFormat { RFC5424, RFC3164 @@ -51,10 +52,10 @@ static enum SyslogFormat { static enum SyslogProtocol { UDP } - - String getDescription(); - IndexerType getIndexerType(); + public String getDescription(); + + public void setCharset(Charset charset); /** * Sends the log data to the Logstash indexer. @@ -64,7 +65,7 @@ static enum SyslogProtocol { * @throws java.io.IOException * The data is not written to the server */ - void push(String data) throws IOException; + public void push(String data) throws IOException; /** * Builds a JSON payload compatible with the Logstash schema. @@ -77,5 +78,5 @@ static enum SyslogProtocol { * The log data to transmit, not null * @return The formatted JSON object, never null */ - JSONObject buildPayload(BuildData buildData, String jenkinsUrl, List logLines); + public JSONObject buildPayload(BuildData buildData, String jenkinsUrl, List logLines); } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java index d49e2ccb..b374a744 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java @@ -39,7 +39,11 @@ * @since 1.0.0 */ public class RabbitMqDao extends AbstractLogstashIndexerDao { - final ConnectionFactory pool; + private final ConnectionFactory pool; + + private String queue; + private String username; + private String password; //primary constructor used by indexer factory public RabbitMqDao(String host, int port, String key, String username, String password) { @@ -47,10 +51,14 @@ public RabbitMqDao(String host, int port, String key, String username, String pa } // Factored for unit testing - RabbitMqDao(ConnectionFactory factory, String host, int port, String key, String username, String password) { - super(host, port, key, username, password); + RabbitMqDao(ConnectionFactory factory, String host, int port, String queue, String username, String password) { + super(host, port); + + this.queue = queue; + this.username = username; + this.password = password; - if (StringUtils.isBlank(key)) { + if (StringUtils.isBlank(queue)) { throw new IllegalArgumentException("rabbit queue name is required"); } @@ -67,6 +75,22 @@ public RabbitMqDao(String host, int port, String key, String username, String pa } } + public String getQueue() + { + return queue; + } + + public String getUsername() + { + return username; + } + + public String getPassword() + { + return password; + } + + @Override public void push(String data) throws IOException { Connection connection = null; @@ -77,28 +101,23 @@ public void push(String data) throws IOException { // Ensure the queue exists try { - channel.queueDeclarePassive(key); + channel.queueDeclarePassive(queue); } catch (IOException e) { // The queue does not exist and the channel has been closed finalizeChannel(channel); // Create the queue channel = connection.createChannel(); - channel.queueDeclare(key, true, false, false, null); + channel.queueDeclare(queue, true, false, false, null); } - channel.basicPublish("", key, null, data.getBytes()); + channel.basicPublish("", queue, null, data.getBytes(getCharset())); } finally { finalizeChannel(channel); finalizeConnection(connection); } } - @Override - public IndexerType getIndexerType() { - return IndexerType.RABBIT_MQ; - } - private void finalizeConnection(Connection connection) { if (connection != null && connection.isOpen()) { try { diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java index b82baab9..9b5677eb 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java @@ -41,16 +41,22 @@ * @since 1.0.0 */ public class RedisDao extends AbstractLogstashIndexerDao { - final JedisPool pool; + private final JedisPool pool; + + private String password; + private String key; //primary constructor used by indexer factory - public RedisDao(String host, int port, String key, String username, String password) { - this(null, host, port, key, username, password); + public RedisDao(String host, int port, String key, String password) { + this(null, host, port, key, password); } // Factored for unit testing - RedisDao(JedisPool factory, String host, int port, String key, String username, String password) { - super(host, port, key, username, password); + RedisDao(JedisPool factory, String host, int port, String key, String password) { + super(host, port); + + this.key = key; + this.password = password; if (StringUtils.isBlank(key)) { throw new IllegalArgumentException("redis key is required"); @@ -61,6 +67,16 @@ public RedisDao(String host, int port, String key, String username, String passw pool = factory == null ? new JedisPool(new JedisPoolConfig(), host, port) : factory; } + public String getPassword() + { + return password; + } + + public String getKey() + { + return key; + } + @Override public void push(String data) throws IOException { Jedis jedis = null; @@ -90,9 +106,4 @@ public void push(String data) throws IOException { } } } - - @Override - public IndexerType getIndexerType() { - return IndexerType.REDIS; - } } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java index de79537c..25a43ae9 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java @@ -1,69 +1,51 @@ package jenkins.plugins.logstash.persistence; -import com.cloudbees.syslog.sender.UdpSyslogMessageSender; +import java.io.IOException; + import com.cloudbees.syslog.Facility; import com.cloudbees.syslog.MessageFormat; import com.cloudbees.syslog.Severity; - -import java.io.IOException; -import java.util.logging.Level; -import java.util.logging.Logger; - -import jenkins.model.Jenkins; -import jenkins.plugins.logstash.LogstashInstallation; -import jenkins.plugins.logstash.LogstashInstallation.Descriptor; +import com.cloudbees.syslog.sender.UdpSyslogMessageSender; public class SyslogDao extends AbstractLogstashIndexerDao { - private String syslogFormat = null; - private final static Logger LOG = Logger.getLogger(SyslogDao.class.getName()); - final UdpSyslogMessageSender messageSender; - - public SyslogDao(String host, int port, String key, String username, String password) { - this(null, host, port, key, username, password); + private MessageFormat messageFormat = MessageFormat.RFC_3164; + private final UdpSyslogMessageSender messageSender; + + public SyslogDao(String host, int port) { + this(null, host, port); } - public SyslogDao(UdpSyslogMessageSender udpSyslogMessageSender, String host, int port, String key, String username, String password) { - super(host, port, key, username, password); + public SyslogDao(UdpSyslogMessageSender udpSyslogMessageSender, String host, int port) { + super(host, port); messageSender = udpSyslogMessageSender == null ? new UdpSyslogMessageSender() : udpSyslogMessageSender; } - public void setSyslogFormat(String format) { - syslogFormat = format; + public void setMessageFormat(MessageFormat format) { + messageFormat = format; + } + + public MessageFormat getMessageFormat() { + return messageFormat; } - + @Override public void push(String data) throws IOException { - - try { - Descriptor logstashPluginConfig = (Descriptor) Jenkins.getInstance().getDescriptor(LogstashInstallation.class); - syslogFormat = logstashPluginConfig.syslogFormat.toString(); - } catch (NullPointerException e){ - LOG.log(Level.WARNING, "Unable to read syslogFormat in the jenkins logstash plugin configuration"); - } // Making the JSON document compliant to Common Event Expression (CEE) // Ref: http://www.rsyslog.com/json-elasticsearch/ data = " @cee: " + data; // SYSLOG Configuration - messageSender.setDefaultMessageHostname(host); + messageSender.setDefaultMessageHostname(getHost()); messageSender.setDefaultAppName("jenkins:"); messageSender.setDefaultFacility(Facility.USER); messageSender.setDefaultSeverity(Severity.INFORMATIONAL); - messageSender.setSyslogServerHostname(host); - messageSender.setSyslogServerPort(port); + messageSender.setSyslogServerHostname(getHost()); + messageSender.setSyslogServerPort(getPort()); // The Logstash syslog input module support only the RFC_3164 format // Ref: https://www.elastic.co/guide/en/logstash/current/plugins-inputs-syslog.html - if (syslogFormat == "RFC3164" ) { - messageSender.setMessageFormat(MessageFormat.RFC_3164); - } - else { - messageSender.setMessageFormat(MessageFormat.RFC_5424); - } + messageSender.setMessageFormat(messageFormat); // Sending the message messageSender.sendMessage(data); } - - @Override - public IndexerType getIndexerType() { return IndexerType.SYSLOG; } } diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/config.jelly b/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/config.jelly new file mode 100644 index 00000000..168397ef --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/config.jelly @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/help-logstashIndexer.html b/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/help-logstashIndexer.html new file mode 100644 index 00000000..86e1fde5 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/LogstashConfiguration/help-logstashIndexer.html @@ -0,0 +1 @@ +The type of Logstash indexer to use. \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly index e58ee543..a8629517 100644 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly +++ b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly @@ -1,33 +1,3 @@ - - - ${it.name()} - - - - - - - - - - - - - - - - - - - ${it.name()} - - - ${it.name()} - - - + diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-key.html b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-key.html deleted file mode 100644 index 3a83bfb0..00000000 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-key.html +++ /dev/null @@ -1,5 +0,0 @@ -
-

REDIS: The name of a Redis list or channel.
- RABBIT_MQ: The name of a RabbitMq queue.
- ELASTICSEARCH: The name and type path. Example: "/indexName/type"

-
diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogProtocol.html b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogProtocol.html deleted file mode 100644 index d9f60024..00000000 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogProtocol.html +++ /dev/null @@ -1,3 +0,0 @@ -
-

The Syslog protocol used to send the messages (only effective for SYSLOG Indexer type).

-
diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-type.html b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-type.html deleted file mode 100644 index a137c868..00000000 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-type.html +++ /dev/null @@ -1,3 +0,0 @@ -
-

The type of Logstash indexer to use.

-
diff --git a/src/main/resources/jenkins/plugins/logstash/Messages.properties b/src/main/resources/jenkins/plugins/logstash/Messages.properties index f2659941..b2844c22 100644 --- a/src/main/resources/jenkins/plugins/logstash/Messages.properties +++ b/src/main/resources/jenkins/plugins/logstash/Messages.properties @@ -23,4 +23,4 @@ DisplayName = Send console log to Logstash ValueIsInt = Value must be an integer ValueIsRequired = Value is required -PleaseProvideHost = Please set a valid host name +PleaseProvideHost = Please set a valid host name \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly new file mode 100644 index 00000000..4113bc28 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-host.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html similarity index 54% rename from src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-host.html rename to src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html index 4e0b7ed4..fdfed43b 100644 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-host.html +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html @@ -1,4 +1,4 @@

The host name or IP address of the indexer to send log data to.
- ELASTICSEARCH: Also specify scheme. Example: "https://myserver".

+ Also specify scheme. Example: "https://myserver".

diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html new file mode 100644 index 00000000..3bb63857 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html @@ -0,0 +1,3 @@ +
+ The name and type path. Example: "/indexName/type" +
diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-password.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-password.html similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-password.html rename to src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-password.html diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-username.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-username.html similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-username.html rename to src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-username.html diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly new file mode 100644 index 00000000..d4292310 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly @@ -0,0 +1,10 @@ + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html b/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html new file mode 100644 index 00000000..4c978cc6 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html @@ -0,0 +1,3 @@ +
+

The host name or IP address of the indexer to send log data to.
+

diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-port.html b/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-port.html similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-port.html rename to src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-port.html diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/configure-advanced.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/configure-advanced.jelly new file mode 100644 index 00000000..eb714659 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/configure-advanced.jelly @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-password.html b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-password.html new file mode 100644 index 00000000..758cb9c5 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-password.html @@ -0,0 +1,4 @@ +
+

The password to use when connecting to the remote indexing server.
+ Leave this field blank if authentication is not enabled.

+
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-queue.html b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-queue.html new file mode 100644 index 00000000..e903cd03 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-queue.html @@ -0,0 +1,3 @@ +
+ The name of a RabbitMq queue. +
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-username.html b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-username.html new file mode 100644 index 00000000..886e0bfa --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/RabbitMq/help-username.html @@ -0,0 +1,4 @@ +
+

The username to use when connecting to the remote indexing server.
+ Leave this field blank if authentication is not enabled or a username is not required.

+
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/Redis/configure-advanced.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/configure-advanced.jelly new file mode 100644 index 00000000..9280df52 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/configure-advanced.jelly @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-key.html b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-key.html new file mode 100644 index 00000000..54217c1d --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-key.html @@ -0,0 +1,3 @@ +
+ The name of a Redis list or channel. +
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-password.html b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-password.html new file mode 100644 index 00000000..758cb9c5 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Redis/help-password.html @@ -0,0 +1,4 @@ +
+

The password to use when connecting to the remote indexing server.
+ Leave this field blank if authentication is not enabled.

+
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/configure-advanced.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/configure-advanced.jelly new file mode 100644 index 00000000..57a244a7 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/configure-advanced.jelly @@ -0,0 +1,9 @@ + + + + ${it.name()} + + + ${it.name()} + + \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogFormat.html b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-messageFormat.html similarity index 68% rename from src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogFormat.html rename to src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-messageFormat.html index 0febfa89..c3c8f6f8 100644 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/help-syslogFormat.html +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-messageFormat.html @@ -1,5 +1,5 @@
-

The Syslog format used to send the messages (only effective for SYSLOG Indexer type).
+

The Syslog format used to send the messages.
RFC5424: The default format.
RFC3164: The format supported by the Logstash Syslog input plugin.

diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-syslogProtocol.html b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-syslogProtocol.html new file mode 100644 index 00000000..8a22e75d --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/Syslog/help-syslogProtocol.html @@ -0,0 +1,3 @@ +
+

The Syslog protocol used to send the messages.

+
diff --git a/src/test/java/jenkins/plugins/logstash/LogstashBuildWrapperTest.java b/src/test/java/jenkins/plugins/logstash/LogstashBuildWrapperTest.java index 08ea164c..30011819 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashBuildWrapperTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashBuildWrapperTest.java @@ -18,7 +18,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class LogstashBuildWrapperTest { diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java new file mode 100644 index 00000000..7f8a8f0c --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java @@ -0,0 +1,133 @@ +package jenkins.plugins.logstash; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; + +import java.io.File; + +import org.hamcrest.core.IsInstanceOf; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mock; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import com.cloudbees.syslog.MessageFormat; + +import jenkins.plugins.logstash.LogstashInstallation.Descriptor; +import jenkins.plugins.logstash.configuration.ElasticSearch; +import jenkins.plugins.logstash.configuration.LogstashIndexer; +import jenkins.plugins.logstash.configuration.RabbitMq; +import jenkins.plugins.logstash.configuration.Redis; +import jenkins.plugins.logstash.configuration.Syslog; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; +import jenkins.plugins.logstash.persistence.LogstashIndexerDao.SyslogFormat; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({ LogstashInstallation.class, Descriptor.class }) +@PowerMockIgnore({"javax.crypto.*"}) +public class LogstashConfigurationMigrationTest extends LogstashConfigurationTestBase +{ + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Mock + Descriptor descriptor; + + LogstashConfiguration configuration; + + @Before + public void setup() + { + mockStatic(LogstashInstallation.class); + configFile = new File("notExisting.xml"); + when(LogstashInstallation.getLogstashDescriptor()).thenReturn(descriptor); + when(descriptor.getHost()).thenReturn("localhost"); + when(descriptor.getPort()).thenReturn(4567); + when(descriptor.getKey()).thenReturn("logstash"); + when(descriptor.getUsername()).thenReturn("user"); + when(descriptor.getPassword()).thenReturn("pwd"); + configuration = new LogstashConfigurationForTest(); + } + + @Test + public void test_redisMigration() + { + when(descriptor.getType()).thenReturn(IndexerType.REDIS); + configuration.migrateData(); + LogstashIndexer indexer = configuration.getLogstashIndexer(); + assertThat(indexer, IsInstanceOf.instanceOf(Redis.class)); + Redis redis = (Redis) indexer; + assertThat(redis.getHost(),equalTo("localhost")); + assertThat(redis.getPort(),is(4567)); + assertThat(redis.getKey(), equalTo("logstash")); + assertThat(redis.getPassword(), equalTo("pwd")); + } + + @Test + public void test_syslogMigrationRFC3164() + { + when(descriptor.getType()).thenReturn(IndexerType.SYSLOG); + when(descriptor.getSyslogFormat()).thenReturn(SyslogFormat.RFC3164); + configuration.migrateData(); + LogstashIndexer indexer = configuration.getLogstashIndexer(); + assertThat(indexer, IsInstanceOf.instanceOf(Syslog.class)); + Syslog syslog = (Syslog) indexer; + assertThat(syslog.getHost(),equalTo("localhost")); + assertThat(syslog.getPort(),is(4567)); + assertThat(syslog.getMessageFormat(), equalTo(MessageFormat.RFC_3164)); + } + + @Test + public void test_syslogMigrationRFC5424() + { + when(descriptor.getType()).thenReturn(IndexerType.SYSLOG); + when(descriptor.getSyslogFormat()).thenReturn(SyslogFormat.RFC5424); + configuration.migrateData(); + LogstashIndexer indexer = configuration.getLogstashIndexer(); + assertThat(indexer, IsInstanceOf.instanceOf(Syslog.class)); + Syslog syslog = (Syslog) indexer; + assertThat(syslog.getHost(),equalTo("localhost")); + assertThat(syslog.getPort(),is(4567)); + assertThat(syslog.getMessageFormat(), equalTo(MessageFormat.RFC_5424)); + } + + @Test + public void test_elasticSearchMigration() + { + when(descriptor.getType()).thenReturn(IndexerType.ELASTICSEARCH); + configuration.migrateData(); + LogstashIndexer indexer = configuration.getLogstashIndexer(); + assertThat(indexer, IsInstanceOf.instanceOf(ElasticSearch.class)); + ElasticSearch es = (ElasticSearch) indexer; + assertThat(es.getHost(),equalTo("localhost")); + assertThat(es.getPort(),is(4567)); + assertThat(es.getKey(), equalTo("logstash")); + assertThat(es.getPassword(), equalTo("pwd")); + assertThat(es.getUsername(), equalTo("user")); + } + + @Test + public void test_rabbitMqMigration() + { + when(descriptor.getType()).thenReturn(IndexerType.RABBIT_MQ); + configuration.migrateData(); + LogstashIndexer indexer = configuration.getLogstashIndexer(); + assertThat(indexer, IsInstanceOf.instanceOf(RabbitMq.class)); + RabbitMq es = (RabbitMq) indexer; + assertThat(es.getHost(),equalTo("localhost")); + assertThat(es.getPort(),is(4567)); + assertThat(es.getQueue(), equalTo("logstash")); + assertThat(es.getPassword(), equalTo("pwd")); + assertThat(es.getUsername(), equalTo("user")); + } + +} diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java new file mode 100644 index 00000000..cb73c3de --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java @@ -0,0 +1,59 @@ +package jenkins.plugins.logstash; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import java.io.File; + +import org.hamcrest.core.IsInstanceOf; +import org.junit.Test; + +import jenkins.plugins.logstash.persistence.ElasticSearchDao; +import jenkins.plugins.logstash.persistence.RabbitMqDao; +import jenkins.plugins.logstash.persistence.RedisDao; +import jenkins.plugins.logstash.persistence.SyslogDao; + +public class LogstashConfigurationTest extends LogstashConfigurationTestBase +{ + + @Test + public void test_unconfiguredWillReturnNull() + { + LogstashConfigurationTestBase.configFile = new File("src/test/resources/notExisting.xml"); + LogstashConfiguration configuration = new LogstashConfigurationForTest(); + assertThat(configuration.getIndexerInstance(), equalTo(null)); + } + + @Test + public void test_elasticSearchIsProperlyConfigured() + { + LogstashConfigurationTestBase.configFile = new File("src/test/resources/elasticSearch.xml"); + LogstashConfiguration configuration = new LogstashConfigurationForTest(); + assertThat(configuration.getIndexerInstance(), IsInstanceOf.instanceOf(ElasticSearchDao.class)); + } + + @Test + public void test_rabbitMqIsProperlyConfigured() + { + LogstashConfigurationTestBase.configFile = new File("src/test/resources/rabbitmq.xml"); + LogstashConfiguration configuration = new LogstashConfigurationForTest(); + assertThat(configuration.getIndexerInstance(), IsInstanceOf.instanceOf(RabbitMqDao.class)); + } + + @Test + public void test_redisIsProperlyConfigured() + { + LogstashConfigurationTestBase.configFile = new File("src/test/resources/redis.xml"); + LogstashConfiguration configuration = new LogstashConfigurationForTest(); + assertThat(configuration.getIndexerInstance(), IsInstanceOf.instanceOf(RedisDao.class)); + } + + @Test + public void test_syslogIsProperlyConfigured() + { + LogstashConfigurationTestBase.configFile = new File("src/test/resources/syslog.xml"); + LogstashConfiguration configuration = new LogstashConfigurationForTest(); + assertThat(configuration.getIndexerInstance(), IsInstanceOf.instanceOf(SyslogDao.class)); + } +} diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTestBase.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTestBase.java new file mode 100644 index 00000000..19ffa704 --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTestBase.java @@ -0,0 +1,26 @@ +package jenkins.plugins.logstash; + +import java.io.File; + +import hudson.XmlFile; + +public class LogstashConfigurationTestBase +{ + protected static File configFile; + + public static class LogstashConfigurationForTest extends LogstashConfiguration + { + + @Override + public synchronized void save() + { + } + + @Override + protected XmlFile getConfigFile() + { + return new XmlFile(configFile); + } + } + +} diff --git a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java index 36827135..7de58a59 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java @@ -5,8 +5,6 @@ import static org.junit.Assert.assertThat; import static org.powermock.api.mockito.PowerMockito.when; -import java.io.IOException; -import java.util.ArrayList; import java.util.List; import org.junit.Before; @@ -14,6 +12,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mock; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; @@ -24,20 +23,20 @@ import hudson.model.Result; import hudson.model.Slave; import hudson.model.queue.QueueTaskFuture; -import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; -import jenkins.plugins.logstash.persistence.IndexerDaoFactory; -import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; -import net.sf.json.JSONArray; +import jenkins.plugins.logstash.persistence.MemoryDao; import net.sf.json.JSONObject; @RunWith(PowerMockRunner.class) @PowerMockIgnore({"javax.crypto.*"}) -@PrepareForTest(IndexerDaoFactory.class) +@PrepareForTest(LogstashConfiguration.class) public class LogstashIntegrationTest { @Rule public JenkinsRule jenkins = new JenkinsRule(); + @Mock + private LogstashConfiguration logstashConfiguration; + private Slave slave; private FreeStyleProject project; @@ -47,22 +46,14 @@ public class LogstashIntegrationTest @Before public void setup() throws Exception { - PowerMockito.mockStatic(IndexerDaoFactory.class); - LogstashInstallation.Descriptor descriptor = LogstashInstallation.getLogstashDescriptor(); - descriptor.type = IndexerType.SYSLOG; - descriptor.host = "localhost"; - descriptor.port = 1; - descriptor.username = "username"; - descriptor.password = "password"; - descriptor.key = "key"; - - memoryDao = new MemoryDao(); - when(IndexerDaoFactory.getInstance(IndexerType.SYSLOG, descriptor.host, descriptor.port, descriptor.key, - descriptor.username, descriptor.password)).thenReturn(memoryDao); + memoryDao = new MemoryDao("localhost", 4567); + PowerMockito.mockStatic(LogstashConfiguration.class); + when(LogstashConfiguration.getInstance()).thenReturn(logstashConfiguration); + when(logstashConfiguration.getIndexerInstance()).thenReturn(memoryDao); + slave = jenkins.createSlave(); slave.setLabelString("myLabel"); project = jenkins.createFreeStyleProject(); - } @Test @@ -131,33 +122,4 @@ public void test_buildNotifierOnSlave() throws Exception assertThat(data.getString("buildHost"),equalTo(slave.getDisplayName())); assertThat(data.getString("buildLabel"),equalTo(slave.getLabelString())); } - - private static class MemoryDao extends AbstractLogstashIndexerDao - { - List output = new ArrayList<>(); - - public MemoryDao() - { - super("localhost", 1, "key", "username", "password"); - } - - @Override - public IndexerType getIndexerType() - { - // TODO Auto-generated method stub - return null; - } - - @Override - public void push(String data) throws IOException - { - JSONObject json = JSONObject.fromObject(data); - output.add(json); - } - - public List getOutput() - { - return output; - } - } } diff --git a/src/test/java/jenkins/plugins/logstash/LogstashOutputStreamTest.java b/src/test/java/jenkins/plugins/logstash/LogstashOutputStreamTest.java index 5b445916..13c4774f 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashOutputStreamTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashOutputStreamTest.java @@ -7,6 +7,7 @@ import java.io.ByteArrayOutputStream; import java.io.OutputStream; +import java.nio.charset.Charset; import org.junit.After; import org.junit.Before; @@ -14,7 +15,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; @SuppressWarnings("resource") @RunWith(MockitoJUnitRunner.class) @@ -32,6 +33,7 @@ public void before() throws Exception { buffer = new ByteArrayOutputStream(); Mockito.doNothing().when(mockWriter).write(anyString()); when(mockWriter.isConnectionBroken()).thenReturn(false); + when(mockWriter.getCharset()).thenReturn(Charset.defaultCharset()); } @After @@ -61,6 +63,7 @@ public void eolSuccess() throws Exception { assertEquals("Results don't match", msg, buffer.toString()); verify(mockWriter).isConnectionBroken(); verify(mockWriter).write(msg); + verify(mockWriter).getCharset(); } @Test @@ -102,6 +105,7 @@ public void eolSuccessConnectionBroken() throws Exception { //Verify calls were made to the dao logging twice, not three times. verify(mockWriter, times(2)).write(msg); verify(mockWriter, times(3)).isConnectionBroken(); + verify(mockWriter, times(2)).getCharset(); } @Test diff --git a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java index 735a6178..35382628 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java @@ -10,7 +10,6 @@ import hudson.tasks.test.AbstractTestResultAction; import jenkins.plugins.logstash.persistence.BuildData; import jenkins.plugins.logstash.persistence.LogstashIndexerDao; -import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; import net.sf.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -22,6 +21,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.Collections; import java.util.GregorianCalendar; @@ -39,19 +39,15 @@ static LogstashWriter createLogstashWriter(final AbstractBuild testBuild, final String url, final LogstashIndexerDao indexer, final BuildData data) { - return new LogstashWriter(testBuild, error, null) { + return new LogstashWriter(testBuild, error, null, testBuild.getCharset()) { @Override - LogstashIndexerDao getDao() throws InstantiationException { - if (indexer == null) { - throw new InstantiationException("DoaTestInstantiationException"); - } - + LogstashIndexerDao getIndexerDao() { return indexer; } @Override BuildData getBuildData() { - assertNotNull("BuildData should never be requested for missing dao.", this.dao); + assertNotNull("BuildData should never be requested for missing dao.", this.getDao()); // For testing, providing null data means use the actual method if (data == null) { @@ -100,6 +96,7 @@ public void before() throws Exception { when(mockBuild.getLog(3)).thenReturn(Arrays.asList("line 1", "line 2", "line 3", "Log truncated...")); when(mockBuild.getEnvironment(null)).thenReturn(new EnvVars()); when(mockBuild.getExecutor()).thenReturn(mockExecutor); + when(mockBuild.getCharset()).thenReturn(Charset.defaultCharset()); when(mockExecutor.getOwner()).thenReturn(mockComputer); when(mockComputer.getNode()).thenReturn(null); @@ -116,7 +113,6 @@ public void before() throws Exception { .thenReturn(JSONObject.fromObject("{\"data\":{},\"message\":[\"test\"],\"source\":\"jenkins\",\"source_host\":\"http://my-jenkins-url\",\"@version\":1}")); Mockito.doNothing().when(mockDao).push(Matchers.anyString()); - when(mockDao.getIndexerType()).thenReturn(IndexerType.REDIS); when(mockDao.getDescription()).thenReturn("localhost:8080"); errorBuffer = new ByteArrayOutputStream(); @@ -156,6 +152,7 @@ public void constructorSuccess() throws Exception { verify(mockBuild).getSensitiveBuildVariables(); verify(mockBuild).getEnvironments(); verify(mockBuild).getEnvironment(null); + verify(mockBuild).getCharset(); verify(mockTestResultAction).getTotalCount(); verify(mockTestResultAction).getSkipCount(); @@ -171,8 +168,7 @@ public void constructorSuccess() throws Exception { @Test public void constructorSuccessNoDao() throws Exception { - String exMessage = "InstantiationException: DoaTestInstantiationException\n" + - "[logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.\n"; + String exMessage = "[logstash-plugin]: Unable to instantiate LogstashIndexerDao with current configuration.\n"; // Unit under test LogstashWriter writer = createLogstashWriter(mockBuild, errorBuffer, "http://my-jenkins-url", null, null); @@ -180,6 +176,7 @@ public void constructorSuccessNoDao() throws Exception { // Verify results assertEquals("Results don't match", exMessage, errorBuffer.toString()); assertTrue("Connection not broken", writer.isConnectionBroken()); + verify(mockBuild).getCharset(); } @Test @@ -196,6 +193,7 @@ public void writeSuccessNoDao() throws Exception { // Verify results assertEquals("Results don't match", "", errorBuffer.toString()); assertTrue("Connection not broken", writer.isConnectionBroken()); + verify(mockBuild).getCharset(); } @Test @@ -211,6 +209,7 @@ public void writeBuildLogSuccessNoDao() throws Exception { // Verify results assertEquals("Results don't match", "", errorBuffer.toString()); assertTrue("Connection not broken", writer.isConnectionBroken()); + verify(mockBuild).getCharset(); } @Test @@ -228,6 +227,7 @@ public void writeSuccess() throws Exception { verify(mockDao).buildPayload(Matchers.eq(mockBuildData), Matchers.eq("http://my-jenkins-url"), Matchers.anyListOf(String.class)); verify(mockDao).push("{\"data\":{},\"message\":[\"test\"],\"source\":\"jenkins\",\"source_host\":\"http://my-jenkins-url\",\"@version\":1}"); + verify(mockBuild).getCharset(); } @Test @@ -242,6 +242,7 @@ public void writeBuildLogSuccess() throws Exception { // No error output assertEquals("Results don't match", "", errorBuffer.toString()); verify(mockBuild).getLog(3); + verify(mockBuild).getCharset(); verify(mockDao).buildPayload(Matchers.eq(mockBuildData), Matchers.eq("http://my-jenkins-url"), Matchers.anyListOf(String.class)); verify(mockDao).push("{\"data\":{},\"message\":[\"test\"],\"source\":\"jenkins\",\"source_host\":\"http://my-jenkins-url\",\"@version\":1}"); @@ -254,7 +255,7 @@ public void writeSuccessConnectionBroken() throws Exception { String msg = "test"; - String exMessage = "[logstash-plugin]: Failed to send log data to REDIS:localhost:8080.\n" + + String exMessage = "[logstash-plugin]: Failed to send log data: localhost:8080.\n" + "[logstash-plugin]: No Further logs will be sent to localhost:8080.\n" + "java.io.IOException: BOOM!"; @@ -287,8 +288,8 @@ public void writeSuccessConnectionBroken() throws Exception { //Verify calls were made to the dao logging twice, not three times. verify(mockDao, times(2)).buildPayload(Matchers.eq(mockBuildData), Matchers.eq("http://my-jenkins-url"), Matchers.anyListOf(String.class)); verify(mockDao, times(2)).push("{\"data\":{},\"message\":[\"test\"],\"source\":\"jenkins\",\"source_host\":\"http://my-jenkins-url\",\"@version\":1}"); - verify(mockDao).getIndexerType(); verify(mockDao, times(2)).getDescription(); + verify(mockBuild).getCharset(); } @Test @@ -304,6 +305,7 @@ public void writeBuildLogGetLogError() throws Exception { // Verify results verify(mockBuild).getLog(3); + verify(mockBuild).getCharset(); List expectedErrorLines = Arrays.asList( "[logstash-plugin]: Unable to serialize log data.", diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java new file mode 100644 index 00000000..2f0c6b1a --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -0,0 +1,62 @@ +package jenkins.plugins.logstash.configuration; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import jenkins.plugins.logstash.persistence.ElasticSearchDao; + +public class ElasticSearchTest +{ + + @Rule + public JenkinsRule j = new JenkinsRule(); + private ElasticSearchDao dao; + + private ElasticSearch indexer; + + @Before + public void setup() + { + indexer = new ElasticSearch(); + indexer.setHost("http://localhost"); + indexer.setPort(4567); + indexer.setPassword("password"); + indexer.setUsername("user"); + indexer.setKey("key"); + dao = indexer.getInstance(); + } + + @Test + public void test_noChangeReturnsSameInstance() + { + assertThat(indexer.shouldRefreshInstance(), is(false)); + assertThat(indexer.getInstance(),is(dao)); + } + + @Test + public void test_passwordChangeLeadsToNewInstance() + { + indexer.setPassword("newPassword"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_usernameChangeLeadsToNewInstance() + { + indexer.setUsername("newUser"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_KeyChangeLeadsToNewInstance() + { + indexer.setKey("newKey"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + +} diff --git a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java new file mode 100644 index 00000000..133fb43b --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java @@ -0,0 +1,54 @@ +package jenkins.plugins.logstash.configuration; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Before; +import org.junit.Test; + +import com.cloudbees.syslog.MessageFormat; + +import jenkins.plugins.logstash.persistence.MemoryDao; + +public class LogstashIndexerTest +{ + + private LogstashIndexerForTest indexer; + + @Before + public void setup() + { + indexer = new LogstashIndexerForTest("localhost", 4567); + indexer.getInstance(); + } + + @Test + public void test_HostChangeLeadsToNewInstance() + { + indexer.setHost("remoteHost"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_PortChangeLeadsToNewInstance() + { + indexer.setPort(7654); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + public static class LogstashIndexerForTest extends LogstashIndexer + { + + public LogstashIndexerForTest(String host, int port) + { + setHost(host); + setPort(port); + } + + @Override + public MemoryDao createIndexerInstance() + { + return new MemoryDao(getHost(), getPort()); + } + } +} diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java new file mode 100644 index 00000000..f398d107 --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java @@ -0,0 +1,62 @@ +package jenkins.plugins.logstash.configuration; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import jenkins.plugins.logstash.persistence.RabbitMqDao; + +public class RabbitMqTest +{ + + @Rule + public JenkinsRule j = new JenkinsRule(); + + private RabbitMq indexer; + private RabbitMqDao dao; + + @Before + public void setup() + { + indexer = new RabbitMq(); + indexer.setHost("localhost"); + indexer.setPort(4567); + indexer.setPassword("password"); + indexer.setUsername("user"); + indexer.setQueue("queue"); + dao = indexer.getInstance(); + } + + @Test + public void test_noChangeReturnsSameInstance() + { + assertThat(indexer.shouldRefreshInstance(), is(false)); + assertThat(indexer.getInstance(),is(dao)); + } + + @Test + public void test_passwordChangeLeadsToNewInstance() + { + indexer.setPassword("newPassword"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_usernameChangeLeadsToNewInstance() + { + indexer.setUsername("newUser"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_QueueChangeLeadsToNewInstance() + { + indexer.setQueue("newQueue"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + +} diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java new file mode 100644 index 00000000..b938e405 --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java @@ -0,0 +1,56 @@ +package jenkins.plugins.logstash.configuration; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import jenkins.plugins.logstash.persistence.RedisDao; + +public class RedisTest +{ + + @Rule + public JenkinsRule j = new JenkinsRule(); + + private Redis indexer; + private RedisDao dao; + + @Before + public void setup() + { + indexer = new Redis(); + indexer.setHost("localhost"); + indexer.setPort(4567); + indexer.setKey("key"); + indexer.setPassword("password"); + dao = indexer.getInstance(); + } + + @Test + public void test_noChangeReturnsSameInstance() + { + assertThat(indexer.shouldRefreshInstance(), is(false)); + assertThat(indexer.getInstance(),is(dao)); + } + + + @Test + public void test_passwordChangeLeadsToNewInstance() + { + indexer.setPassword("newPassword"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + @Test + public void test_KeyChangeLeadsToNewInstance() + { + indexer.setKey("newKey"); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + + +} diff --git a/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java new file mode 100644 index 00000000..2d77b342 --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java @@ -0,0 +1,43 @@ +package jenkins.plugins.logstash.configuration; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Before; +import org.junit.Test; + +import com.cloudbees.syslog.MessageFormat; + +import jenkins.plugins.logstash.persistence.SyslogDao; + +public class SyslogTest +{ + + private Syslog indexer; + private SyslogDao dao; + + @Before + public void setup() + { + indexer = new Syslog(); + indexer.setHost("localhost"); + indexer.setPort(4567); + indexer.setMessageFormat(MessageFormat.RFC_3164); + dao = indexer.getInstance(); + } + + @Test + public void test_noChangeReturnsSameInstance() + { + assertThat(indexer.shouldRefreshInstance(), is(false)); + assertThat(indexer.getInstance(),is(dao)); + } + + @Test + public void test_messageFormatChangeLeadsToNewInstance() + { + indexer.setMessageFormat(MessageFormat.RFC_5424); + assertThat(indexer.shouldRefreshInstance(), is(true)); + } + +} diff --git a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java index eb893e02..e593aca1 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java @@ -4,6 +4,7 @@ import static org.mockito.Mockito.when; import java.io.IOException; +import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; @@ -13,7 +14,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class AbstractLogstashIndexerDaoTest { @@ -66,13 +67,9 @@ public void buildPayloadSuccessTwoLines() throws Exception { } private AbstractLogstashIndexerDao getInstance() { - return new AbstractLogstashIndexerDao("localhost", -1, "", "", "") { - - public IndexerType getIndexerType() { - return IndexerType.REDIS; - } - - public void push(String data) throws IOException {} + return new AbstractLogstashIndexerDao("localhost", -1) { + @Override + public void push(String data) throws IOException {} }; } } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java index 2e445b21..e7150770 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java @@ -239,12 +239,12 @@ public void constructorSuccessTestFailures() throws Exception { // Unit under test BuildData buildData = new BuildData(mockBuild, mockDate, mockListener); - Assert.assertEquals("Incorrect test results", 123, buildData.testResults.totalCount); - Assert.assertEquals("Incorrect test results", 0, buildData.testResults.skipCount); - Assert.assertEquals("Incorrect test results", 1, buildData.testResults.failCount); - Assert.assertEquals("Incorrect test details count", 1, buildData.testResults.failedTestsWithErrorDetail.size()); - Assert.assertEquals("Incorrect failed test error details", "ErrorDetails Test", buildData.testResults.failedTestsWithErrorDetail.get(0).errorDetails); - Assert.assertEquals("Incorrect failed test fullName", "Mock Full Test", buildData.testResults.failedTestsWithErrorDetail.get(0).fullName); + Assert.assertEquals("Incorrect test results", 123, buildData.testResults.getTotalCount()); + Assert.assertEquals("Incorrect test results", 0, buildData.testResults.getSkipCount()); + Assert.assertEquals("Incorrect test results", 1, buildData.testResults.getFailCount()); + Assert.assertEquals("Incorrect test details count", 1, buildData.testResults.getFailedTestsWithErrorDetail().size()); + Assert.assertEquals("Incorrect failed test error details", "ErrorDetails Test", buildData.testResults.getFailedTestsWithErrorDetail().get(0).getErrorDetails()); + Assert.assertEquals("Incorrect failed test fullName", "Mock Full Test", buildData.testResults.getFailedTestsWithErrorDetail().get(0).getFullName()); verifyMocks(); verifyTestResultActions(); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java index 17051a92..83df99b6 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java @@ -14,7 +14,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.*; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -109,13 +109,13 @@ public void constructorSuccess1() throws Exception { dao = createDao("https://localhost", 8200, "logstash", "username", "password"); // Verify results - assertEquals("Wrong host name", "https://localhost", dao.host); - assertEquals("Wrong port", 8200, dao.port); - assertEquals("Wrong key", "logstash", dao.key); - assertEquals("Wrong name", "username", dao.username); - assertEquals("Wrong password", "password", dao.password); - assertEquals("Wrong auth", "dXNlcm5hbWU6cGFzc3dvcmQ=", dao.auth); - assertEquals("Wrong uri", new URI("https://localhost:8200/logstash"), dao.uri); + assertEquals("Wrong host name", "https://localhost", dao.getHost()); + assertEquals("Wrong port", 8200, dao.getPort()); + assertEquals("Wrong key", "logstash", dao.getKey()); + assertEquals("Wrong name", "username", dao.getUsername()); + assertEquals("Wrong password", "password", dao.getPassword()); + assertEquals("Wrong auth", "dXNlcm5hbWU6cGFzc3dvcmQ=", dao.getAuth()); + assertEquals("Wrong uri", new URI("https://localhost:8200/logstash"), dao.getUri()); } @Test @@ -124,13 +124,13 @@ public void constructorSuccess2() throws Exception { dao = createDao("http://localhost", 8200, "jenkins/logstash", "", "password"); // Verify results - assertEquals("Wrong host name", "http://localhost", dao.host); - assertEquals("Wrong port", 8200, dao.port); - assertEquals("Wrong key", "jenkins/logstash", dao.key); - assertEquals("Wrong name", "", dao.username); - assertEquals("Wrong password", "password", dao.password); - assertEquals("Wrong auth", null, dao.auth); - assertEquals("Wrong uri", new URI("http://localhost:8200/jenkins/logstash"), dao.uri); + assertEquals("Wrong host name", "http://localhost", dao.getHost()); + assertEquals("Wrong port", 8200, dao.getPort()); + assertEquals("Wrong key", "jenkins/logstash", dao.getKey()); + assertEquals("Wrong name", "", dao.getUsername()); + assertEquals("Wrong password", "password", dao.getPassword()); + assertEquals("Wrong auth", null, dao.getAuth()); + assertEquals("Wrong uri", new URI("http://localhost:8200/jenkins/logstash"), dao.getUri()); } @Test @@ -139,13 +139,13 @@ public void constructorSuccess3() throws Exception { dao = createDao("http://localhost", 8200, "/jenkins//logstash/", "userlongername", null); // Verify results - assertEquals("Wrong host name", "http://localhost", dao.host); - assertEquals("Wrong port", 8200, dao.port); - assertEquals("Wrong key", "/jenkins//logstash/", dao.key); - assertEquals("Wrong name", "userlongername", dao.username); - assertEquals("Wrong password", null, dao.password); - assertEquals("Wrong auth", "dXNlcmxvbmdlcm5hbWU6", dao.auth); - assertEquals("Wrong uri", new URI("http://localhost:8200/jenkins//logstash/"), dao.uri); + assertEquals("Wrong host name", "http://localhost", dao.getHost()); + assertEquals("Wrong port", 8200, dao.getPort()); + assertEquals("Wrong key", "/jenkins//logstash/", dao.getKey()); + assertEquals("Wrong name", "userlongername", dao.getUsername()); + assertEquals("Wrong password", null, dao.getPassword()); + assertEquals("Wrong auth", "dXNlcmxvbmdlcm5hbWU6", dao.getAuth()); + assertEquals("Wrong uri", new URI("http://localhost:8200/jenkins//logstash/"), dao.getUri()); } @Test diff --git a/src/test/java/jenkins/plugins/logstash/persistence/IndexerDaoFactoryTest.java b/src/test/java/jenkins/plugins/logstash/persistence/IndexerDaoFactoryTest.java deleted file mode 100644 index 408c02d3..00000000 --- a/src/test/java/jenkins/plugins/logstash/persistence/IndexerDaoFactoryTest.java +++ /dev/null @@ -1,43 +0,0 @@ -package jenkins.plugins.logstash.persistence; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import jenkins.plugins.logstash.persistence.LogstashIndexerDao.IndexerType; - -import org.junit.Test; - -public class IndexerDaoFactoryTest { - - @Test - public void getAllInstances() throws Exception { - for (IndexerType type : IndexerType.values()) { - String host = type == IndexerType.ELASTICSEARCH ? "http://localhost" : "localhost"; - LogstashIndexerDao dao = IndexerDaoFactory.getInstance(type, host, 1234, "key", "username", "password"); - - assertNotNull("Result was null", dao); - assertEquals("Result implements wrong IndexerType", type, dao.getIndexerType()); - } - } - - @Test - public void successNulls() throws Exception { - for (IndexerType type : IndexerType.values()) { - String host = type == IndexerType.ELASTICSEARCH ? "http://localhost" : "localhost"; - LogstashIndexerDao dao = IndexerDaoFactory.getInstance(type, host, null, "key", null, null); - - assertNotNull("Result was null", dao); - assertEquals("Result implements wrong IndexerType", type, dao.getIndexerType()); - } - } - - @Test(expected = InstantiationException.class) - public void failureNullType() throws Exception { - try { - IndexerDaoFactory.getInstance(null, "localhost", 1234, "key", "username", "password"); - } catch (InstantiationException e) { - String msg = "[logstash-plugin]: Unknown IndexerType 'null'. Did you forget to configure the plugin?"; - assertEquals("Wrong message", msg, e.getMessage()); - throw e; - } - } -} diff --git a/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java b/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java new file mode 100644 index 00000000..502c6b7f --- /dev/null +++ b/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java @@ -0,0 +1,30 @@ +package jenkins.plugins.logstash.persistence; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; +import net.sf.json.JSONObject; + +public class MemoryDao extends AbstractLogstashIndexerDao +{ + List output = new ArrayList<>(); + + public MemoryDao(String host, int port) + { + super(host, port); + } + + @Override + public void push(String data) throws IOException + { + JSONObject json = JSONObject.fromObject(data); + output.add(json); + } + + public List getOutput() + { + return output; + } +} \ No newline at end of file diff --git a/src/test/java/jenkins/plugins/logstash/persistence/RabbitMqDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/RabbitMqDaoTest.java index b0f8d183..acdfd523 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/RabbitMqDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/RabbitMqDaoTest.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.net.SocketException; +import java.nio.charset.Charset; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.exception.ExceptionUtils; @@ -13,7 +14,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import com.rabbitmq.client.AuthenticationFailureException; import com.rabbitmq.client.Channel; @@ -45,6 +46,7 @@ public void before() throws Exception { int port = (int) (Math.random() * 1000); // Note that we can't run these tests in parallel dao = createDao("localhost", port, "logstash", "username", "password"); + dao.setCharset(Charset.defaultCharset()); when(mockPool.newConnection()).thenReturn(mockConnection); @@ -107,11 +109,11 @@ public void constructorSuccess() throws Exception { dao = createDao("localhost", 5672, "logstash", "username", "password"); // Verify results - assertEquals("Wrong host name", "localhost", dao.host); - assertEquals("Wrong port", 5672, dao.port); - assertEquals("Wrong key", "logstash", dao.key); - assertEquals("Wrong name", "username", dao.username); - assertEquals("Wrong password", "password", dao.password); + assertEquals("Wrong host name", "localhost", dao.getHost()); + assertEquals("Wrong port", 5672, dao.getPort()); + assertEquals("Wrong key", "logstash", dao.getQueue()); + assertEquals("Wrong name", "username", dao.getUsername()); + assertEquals("Wrong password", "password", dao.getPassword()); } @Test(expected = IOException.class) @@ -195,6 +197,7 @@ public void pushSuccess() throws Exception { public void pushSuccessNoAuth() throws Exception { String json = "{ 'foo': 'bar' }"; dao = createDao("localhost", 5672, "logstash", null, null); + dao.setCharset(Charset.defaultCharset()); // Unit under test dao.push(json); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java index 6a1fedbc..53e2efca 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java @@ -27,7 +27,7 @@ public class RedisDaoTest { @Mock Jedis mockJedis; RedisDao createDao(String host, int port, String key, String username, String password) { - return new RedisDao(mockPool, host, port, key, username, password); + return new RedisDao(mockPool, host, port, key, password); } @Before @@ -90,11 +90,10 @@ public void constructorSuccess() throws Exception { dao = createDao("localhost", 6379, "logstash", "username", "password"); // Verify results - assertEquals("Wrong host name", "localhost", dao.host); - assertEquals("Wrong port", 6379, dao.port); - assertEquals("Wrong key", "logstash", dao.key); - assertEquals("Wrong name", "username", dao.username); - assertEquals("Wrong password", "password", dao.password); + assertEquals("Wrong host name", "localhost", dao.getHost()); + assertEquals("Wrong port", 6379, dao.getPort()); + assertEquals("Wrong key", "logstash", dao.getKey()); + assertEquals("Wrong password", "password", dao.getPassword()); } @Test(expected = IOException.class) diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java index 24bf22b3..9637bffc 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java @@ -23,9 +23,9 @@ public class SyslogDaoTest { int port = 514; UdpSyslogMessageSender udpSyslogMessageSender = new UdpSyslogMessageSender(); @Mock UdpSyslogMessageSender mockUdpSyslogMessageSender; - + @Before - public void before() throws Exception { + public void before() throws Exception { dao = createDao(host, port, null, null, null); dao.push(data); } @@ -36,7 +36,7 @@ public void ceeMessageFormat() throws Exception { verify(mockUdpSyslogMessageSender, times(1)).sendMessage(" @cee: " + data); } - // Test the MessageSender configuration. + // Test the MessageSender configuration. @Test public void syslogConfig() throws Exception { verify(mockUdpSyslogMessageSender, times(1)).setDefaultMessageHostname(host); @@ -45,11 +45,11 @@ public void syslogConfig() throws Exception { verify(mockUdpSyslogMessageSender, times(1)).setSyslogServerPort(port); verify(mockUdpSyslogMessageSender, times(1)).setDefaultFacility(Facility.USER); verify(mockUdpSyslogMessageSender, times(1)).setDefaultSeverity(Severity.INFORMATIONAL); - verify(mockUdpSyslogMessageSender, times(1)).setMessageFormat(MessageFormat.RFC_5424); + verify(mockUdpSyslogMessageSender, times(1)).setMessageFormat(MessageFormat.RFC_3164); } - + SyslogDao createDao(String host, int port, String key, String username, String password) { - return new SyslogDao(mockUdpSyslogMessageSender, host, port, key, username, password); + return new SyslogDao(mockUdpSyslogMessageSender, host, port); } - + } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java index 1181d8f3..db6d4223 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java @@ -1,5 +1,6 @@ package jenkins.plugins.logstash.persistence; +import com.cloudbees.syslog.MessageFormat; import com.cloudbees.syslog.sender.UdpSyslogMessageSender; import java.io.FileNotFoundException; import java.io.PrintWriter; @@ -26,9 +27,9 @@ public class SyslogDaoTestIT{ int port = 514; int logstashTimeout = 15; UdpSyslogMessageSender udpSyslogMessageSender = new UdpSyslogMessageSender(); - + @Before - public void before() throws Exception { + public void before() throws Exception { dao = createDao(host, port, null, null, null); } @@ -37,7 +38,7 @@ public void before() throws Exception { public void syslogSendRFC3164UDP() throws Exception { PrintWriter writer = null; - + // Clean up the the logstash log file try { writer = new PrintWriter(logfile); @@ -46,23 +47,23 @@ public void syslogSendRFC3164UDP() throws Exception { fail("Unable to clean up the logstash log file: " + e.getMessage()); } finally { - if (writer != null) { - writer.close(); + if (writer != null) { + writer.close(); } } - - // Send the syslog message - dao.setSyslogFormat("RFC3164"); + + // Send the syslog message + dao.setMessageFormat(MessageFormat.RFC_3164); dao.push(data); - - // Await for logstash to process the message + + // Await for logstash to process the message try { await().atMost(logstashTimeout, TimeUnit.SECONDS).until(logfileIsNotEmpty()); } catch (Exception e) { fail("Unable to find any logstash generated data within the logfile " + logfile + ": " + e.getMessage()); } - + // Parse the logstash generated logfile try { JSONObject logjson = JSONObject.fromObject(logcontent); @@ -71,14 +72,15 @@ public void syslogSendRFC3164UDP() throws Exception { catch (Exception e) { fail("Unable to parse the logstash generated logfile content: " + e.getMessage()); } - - assertEquals(" @cee: " + data, logmessage); - + + assertEquals(" @cee: " + data, logmessage); + } - + private Callable logfileIsNotEmpty() { return new Callable() { - public Boolean call() throws Exception { + @Override + public Boolean call() throws Exception { // Check the content of logfile generated by logstash logcontent = new String(Files.readAllBytes(Paths.get(logfile))); // The condition that must be fulfilled @@ -86,9 +88,9 @@ public Boolean call() throws Exception { } }; } - + SyslogDao createDao(String host, int port, String key, String username, String password) { - return new SyslogDao(udpSyslogMessageSender, host, port, key, username, password); + return new SyslogDao(udpSyslogMessageSender, host, port); } - + } diff --git a/src/test/resources/elasticSearch.xml b/src/test/resources/elasticSearch.xml new file mode 100644 index 00000000..3f3d7c7e --- /dev/null +++ b/src/test/resources/elasticSearch.xml @@ -0,0 +1,11 @@ + + + + http://localhost + 9300 + logstash + + {AQAAABAAAAAQdB13xsx5UlCScGiBcVUOL0GqWYwAU5syhW9iBb6tG+4=} + + true + \ No newline at end of file diff --git a/src/test/resources/rabbitmq.xml b/src/test/resources/rabbitmq.xml new file mode 100644 index 00000000..62497b34 --- /dev/null +++ b/src/test/resources/rabbitmq.xml @@ -0,0 +1,10 @@ + + + localhost + 5672 + logstash + + {AQAAABAAAAAQ5wlElDfWGri9VuaMh0MCBZWs1fjL31zvnxrkszfW5pA=} + + true + \ No newline at end of file diff --git a/src/test/resources/redis.xml b/src/test/resources/redis.xml new file mode 100644 index 00000000..a4cff3a5 --- /dev/null +++ b/src/test/resources/redis.xml @@ -0,0 +1,10 @@ + + + + localhost + 6379 + logstash + {AQAAABAAAAAQAwaAxyveddM0PF+kR0dYFAymdth9PpitQnvJW0SR6JU=} + + true + \ No newline at end of file diff --git a/src/test/resources/syslog.xml b/src/test/resources/syslog.xml new file mode 100644 index 00000000..2c6c0dc4 --- /dev/null +++ b/src/test/resources/syslog.xml @@ -0,0 +1,10 @@ + + + + localhost + 519 + RFC_3164 + UDP + + true + \ No newline at end of file From 333ca413030d51e8bc6202e4a704ee8959637199 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 28 Dec 2017 01:39:01 +0100 Subject: [PATCH 04/17] jenkins and java use Jenkins 2.7.4 instead of 2.60.3 fix problem with cast that is requried with java 7 --- pom.xml | 2 +- .../jenkins/plugins/logstash/configuration/LogstashIndexer.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index abaac5d3..d62eda32 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ UTF-8 true - 2.60.3 + 2.7.4 2.23 diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index 1ca17e9a..f0e52e92 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -112,7 +112,7 @@ protected boolean shouldRefreshInstance() @SuppressWarnings("unchecked") public static DescriptorExtensionList, Descriptor>> all() { - return Jenkins.getInstance().getDescriptorList(LogstashIndexer.class); + return (DescriptorExtensionList) Jenkins.getInstance().getDescriptorList(LogstashIndexer.class); } public static abstract class LogstashIndexerDescriptor extends Descriptor> From ca4eca8bcc95f14b950d72d20516d128e745a2e0 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 28 Dec 2017 22:43:22 +0100 Subject: [PATCH 05/17] fix findbugs issues in buildData Use FastDateFormat from apache which is thread safe Avoid possible NPE in Node detection Avoid possible NPE in result (previous logic already avoided it but findbugs still claimed it to be a problem) Node detection is same for pipeline and freestyle so move it to initData method --- .../logstash/persistence/BuildData.java | 44 +++++++++---------- .../plugins/logstash/LogstashWriterTest.java | 2 +- .../logstash/persistence/BuildDataTest.java | 2 +- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java index edb40760..3a07dc18 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/BuildData.java @@ -26,17 +26,15 @@ import hudson.model.Action; import hudson.model.Environment; +import hudson.model.Executor; import hudson.model.Result; import hudson.model.AbstractBuild; import hudson.model.TaskListener; import hudson.model.Run; import hudson.model.Node; -import hudson.model.Executor; import hudson.tasks.test.AbstractTestResultAction; import hudson.tasks.test.TestResult; -import java.text.DateFormat; -import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Calendar; import java.util.Collections; @@ -53,6 +51,7 @@ import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.time.FastDateFormat; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -65,7 +64,7 @@ */ public class BuildData { // ISO 8601 date format - public transient static final DateFormat DATE_FORMATTER = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); + public transient static final FastDateFormat DATE_FORMATTER = FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); private final static Logger LOGGER = Logger.getLogger(MethodHandles.lookup().lookupClass().getCanonicalName()); public static class TestData { int totalCount, skipCount, failCount, passCount; @@ -138,15 +137,6 @@ public TestData(Action action) { public BuildData(AbstractBuild build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Node node = build.getExecutor().getOwner().getNode(); - if (node == null) { - buildHost = "master"; - buildLabel = "master"; - } else { - buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); - buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); - } - // build.getDuration() is always 0 in Notifiers rootProjectName = build.getRootBuild().getProject().getName(); rootFullProjectName = build.getRootBuild().getProject().getFullName(); @@ -186,15 +176,6 @@ public BuildData(AbstractBuild build, Date currentTime, TaskListener liste public BuildData(Run build, Date currentTime, TaskListener listener) { initData(build, currentTime); - Node node = build.getExecutor().getOwner().getNode(); - if (node == null) { - buildHost = "master"; - buildLabel = "master"; - } else { - buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); - buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); - } - rootProjectName = projectName; rootFullProjectName = fullProjectName; rootProjectDisplayName = displayName; @@ -210,7 +191,24 @@ public BuildData(Run build, Date currentTime, TaskListener listener) { } private void initData(Run build, Date currentTime) { - result = build.getResult() == null ? null : build.getResult().toString(); + + Executor executor = build.getExecutor(); + if (executor == null) { + buildHost = "master"; + buildLabel = "master"; + } else { + Node node = executor.getOwner().getNode(); + if (node == null) { + buildHost = "master"; + buildLabel = "master"; + } else { + buildHost = StringUtils.isBlank(node.getDisplayName()) ? "master" : node.getDisplayName(); + buildLabel = StringUtils.isBlank(node.getLabelString()) ? "master" : node.getLabelString(); + } + } + + Result result = build.getResult(); + this.result = result == null ? null : result.toString(); id = build.getId(); projectName = build.getParent().getName(); fullProjectName = build.getParent().getFullName(); diff --git a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java index 735a6178..3a07f2a3 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashWriterTest.java @@ -139,7 +139,7 @@ public void constructorSuccess() throws Exception { // Verify that the BuildData constructor is what is being called here. // This also lets us verify that in the instantiation failure cases we do not construct BuildData. verify(mockBuild).getId(); - verify(mockBuild, times(2)).getResult(); + verify(mockBuild).getResult(); verify(mockBuild, times(2)).getParent(); verify(mockBuild, times(2)).getProject(); verify(mockBuild, times(1)).getStartTimeInMillis(); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java index 2e445b21..507de593 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/BuildDataTest.java @@ -121,7 +121,7 @@ private void verifyMocks() throws Exception verify(mockProject).getFullName(); verify(mockBuild).getId(); - verify(mockBuild, times(2)).getResult(); + verify(mockBuild).getResult(); verify(mockBuild, times(2)).getParent(); verify(mockBuild).getDisplayName(); verify(mockBuild).getFullDisplayName(); From 132110473f3fbf975e85370b098fe9ddc0d53995 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Mon, 1 Jan 2018 23:38:25 +0100 Subject: [PATCH 06/17] javadoc and readme update javadoc to get more information for developers that want to extend the plugin. Update the readme Use the not deprecated junit runner of mockito --- README.md | 7 ++++--- .../logstash/LogstashConfiguration.java | 9 +++++++++ .../configuration/LogstashIndexer.java | 18 +++++++++++++++++- .../AbstractLogstashIndexerDaoTest.java | 1 - .../logstash/persistence/RedisDaoTest.java | 2 +- .../logstash/persistence/SyslogDaoTest.java | 2 +- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 81317be9..9ac7bb30 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ Contributing Adding support for new indexers ------------------------------- -* Create a new class in the package `jenkins.plugins.logstash.persistence` that extends `AbstractLogstashIndexerDao` -* Add a new entry to the enum `IndexerType` in `LogstashIndexerDao` -* Add a new mapping to the `INDEXER_MAP` in `IndexerDaoFactory` +* Implement the extension point `jenkins.plugins.logstash.configuration.LogstashIndexer` that will take your configuration. +Override the method `shouldRefreshInstance()` where you decide if a new dao instance must be created because the configuration has changed in the meantime. +* Create a `configure-advanced.jelly` for the UI part of your configuration. +* Create a new class that extends `jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao`. This class will do the actual work of pushing the logs to the indexer. diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 5901cd56..0d60b69d 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -36,6 +36,11 @@ public LogstashConfiguration() load(); } + /** + * Returns the current logstash indexer configuration. + * + * @return configuration instance + */ public LogstashIndexer getLogstashIndexer() { return logstashIndexer; @@ -46,6 +51,10 @@ public void setLogstashIndexer(LogstashIndexer logstashIndexer) this.logstashIndexer = logstashIndexer; } + /** + * Returns the actual instance of the logstash dao. + * @return dao instance + */ @CheckForNull public LogstashIndexerDao getIndexerInstance() { diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index f0e52e92..dcd43e89 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -19,6 +19,13 @@ import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; import net.sf.json.JSONObject; +/** + * Extension point for logstash indexers. + * This extension point provides the configuration for the indexer. You also have to implement the actual + * indexer in a separate class extending {@link AbstractLogstashIndexerDao}. + * + * @param The class implementing the push to the indexer + */ public abstract class LogstashIndexer extends AbstractDescribableImpl> implements ExtensionPoint, ReconfigurableDescribable> @@ -75,7 +82,8 @@ public void setPort(int port) /** * Gets the instance of the actual {@link AbstractLogstashIndexerDao} that is represented by this * configuration. - * When changing the configuration a new instance will be created. + * Checks via {@link #shouldRefreshInstance()}, if a new {@link AbstractLogstashIndexerDao} + * needs to be created. If yes calls {@link #createIndexerInstance()} to create a new instance. * * @return {@link AbstractLogstashIndexerDao} instance */ @@ -97,6 +105,11 @@ public final T getInstance() */ protected abstract T createIndexerInstance(); + /** + * Decides whether a new instance of {@link AbstractLogstashIndexerDao} should be created or not. + * Implementers should overwrite this method if they have own configuration. + * @return true if the current configuration differs from the current {@link AbstractLogstashIndexerDao dao instance} + */ protected boolean shouldRefreshInstance() { if (instance == null) @@ -147,6 +160,9 @@ public FormValidation doCheckHost(@QueryParameter("value") String value) public abstract int getDefaultPort(); } + /** + * {@inheritDoc} + */ @Override public LogstashIndexer reconfigure(StaplerRequest req, JSONObject form) throws FormException { diff --git a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java index e593aca1..7ab763d0 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java @@ -4,7 +4,6 @@ import static org.mockito.Mockito.when; import java.io.IOException; -import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; diff --git a/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java index 53e2efca..a666a508 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/RedisDaoTest.java @@ -12,7 +12,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java index 9637bffc..f37b44c3 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java @@ -7,7 +7,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import com.cloudbees.syslog.Facility; import com.cloudbees.syslog.MessageFormat; From 49742763ab15e018694c7feddac13e5b81389d78 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Mon, 1 Jan 2018 23:46:38 +0100 Subject: [PATCH 07/17] remove unnecessary dependency to workflow-step-api --- pom.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pom.xml b/pom.xml index 85ed9524..ae28f223 100644 --- a/pom.xml +++ b/pom.xml @@ -139,11 +139,6 @@ 2.0.0-beta.5 test - - org.jenkins-ci.plugins.workflow - workflow-step-api - 1.15 - From 227611b7acb11ea846fe9cb53adbea916dd57d57 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 3 Jan 2018 02:23:04 +0100 Subject: [PATCH 08/17] do the comparison if something has changed in the configuration class --- .../logstash/LogstashConfiguration.java | 23 +++++---- .../logstash/configuration/ElasticSearch.java | 47 +++++++++++++++++-- .../configuration/LogstashIndexer.java | 44 ++++++++++------- .../logstash/configuration/RabbitMq.java | 47 +++++++++++++++++-- .../plugins/logstash/configuration/Redis.java | 36 ++++++++++++-- .../logstash/configuration/Syslog.java | 26 ++++++++-- .../configuration/ElasticSearchTest.java | 31 ++++++------ .../configuration/LogstashIndexerTest.java | 19 +++++--- .../logstash/configuration/RabbitMqTest.java | 29 +++++++----- .../logstash/configuration/RedisTest.java | 27 ++++++----- .../logstash/configuration/SyslogTest.java | 21 +++++---- 11 files changed, 247 insertions(+), 103 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 0d60b69d..4579df82 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -30,10 +30,15 @@ public class LogstashConfiguration extends GlobalConfiguration private static final Logger LOGGER = Logger.getLogger(LogstashConfiguration.class.getName()); private LogstashIndexer logstashIndexer; private boolean dataMigrated = false; + private transient LogstashIndexer activeIndexer; public LogstashConfiguration() { load(); + if (logstashIndexer != null) + { + activeIndexer = logstashIndexer; + } } /** @@ -58,9 +63,9 @@ public void setLogstashIndexer(LogstashIndexer logstashIndexer) @CheckForNull public LogstashIndexerDao getIndexerInstance() { - if (logstashIndexer != null) + if (activeIndexer != null) { - return logstashIndexer.getInstance(); + return activeIndexer.getInstance(); } return null; } @@ -134,6 +139,7 @@ public void migrateData() LOGGER.log(Level.INFO, "unknown logstash Indexer type: " + type); break; } + activeIndexer = logstashIndexer; } dataMigrated = true; save(); @@ -143,15 +149,12 @@ public void migrateData() @Override public boolean configure(StaplerRequest staplerRequest, JSONObject json) throws FormException { - JSONObject j = json.getJSONObject("logstashIndexer"); - String clazz = j.getString("stapler-class"); - if (logstashIndexer == null || !logstashIndexer.getClass().getName().equals(clazz)) - { - staplerRequest.bindJSON(this, json); - } - else + // when we bind the stapler request we get a new instance of logstashIndexer + staplerRequest.bindJSON(this, json); + + if (logstashIndexer != null && !logstashIndexer.equals(activeIndexer)) { - logstashIndexer.reconfigure(staplerRequest, j); + activeIndexer = logstashIndexer; } save(); return true; diff --git a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java index 8045ac24..2add16a7 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java @@ -59,12 +59,49 @@ public void setPassword(String password) } @Override - protected boolean shouldRefreshInstance() + public boolean equals(Object obj) { - return super.shouldRefreshInstance() || - !instance.getPassword().equals(Secret.toString(password)) || - !StringUtils.equals(instance.getUsername(), username) || - !StringUtils.equals(instance.getKey(), key); + if (this == obj) + return true; + if (!super.equals(obj)) + return false; + if (getClass() != obj.getClass()) + return false; + ElasticSearch other = (ElasticSearch) obj; + if (!Secret.toString(password).equals(other.getPassword())) + { + return false; + } + if (key == null) + { + if (other.key != null) + return false; + } + else if (!key.equals(other.key)) + { + return false; + } + if (username == null) + { + if (other.username != null) + return false; + } + else if (!username.equals(other.username)) + { + return false; + } + return true; + } + + @Override + public int hashCode() + { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((key == null) ? 0 : key.hashCode()); + result = prime * result + ((username == null) ? 0 : username.hashCode()); + result = prime * result + Secret.toString(password).hashCode(); + return result; } @Override diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index dcd43e89..03935e94 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -82,15 +82,13 @@ public void setPort(int port) /** * Gets the instance of the actual {@link AbstractLogstashIndexerDao} that is represented by this * configuration. - * Checks via {@link #shouldRefreshInstance()}, if a new {@link AbstractLogstashIndexerDao} - * needs to be created. If yes calls {@link #createIndexerInstance()} to create a new instance. * * @return {@link AbstractLogstashIndexerDao} instance */ @Nonnull public final T getInstance() { - if (shouldRefreshInstance()) + if (instance == null) { instance = createIndexerInstance(); } @@ -105,23 +103,35 @@ public final T getInstance() */ protected abstract T createIndexerInstance(); - /** - * Decides whether a new instance of {@link AbstractLogstashIndexerDao} should be created or not. - * Implementers should overwrite this method if they have own configuration. - * @return true if the current configuration differs from the current {@link AbstractLogstashIndexerDao dao instance} - */ - protected boolean shouldRefreshInstance() - { - if (instance == null) - { - return true; - } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((host == null) ? 0 : host.hashCode()); + result = prime * result + port; + return result; + } - boolean matches = StringUtils.equals(instance.getHost(), host) && - (instance.getPort() == port); - return !matches; + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + LogstashIndexer other = (LogstashIndexer) obj; + if (host == null) { + if (other.host != null) + return false; + } else if (!host.equals(other.host)) + return false; + if (port != other.port) + return false; + return true; } + @SuppressWarnings("unchecked") public static DescriptorExtensionList, Descriptor>> all() { diff --git a/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java index 3dce6418..48ed671f 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java @@ -57,12 +57,49 @@ public void setPassword(String password) } @Override - protected boolean shouldRefreshInstance() + public boolean equals(Object obj) { - return super.shouldRefreshInstance() || - !instance.getPassword().equals(Secret.toString(password)) || - !StringUtils.equals(instance.getUsername(), username) || - !StringUtils.equals(instance.getQueue(), queue); + if (this == obj) + return true; + if (!super.equals(obj)) + return false; + if (getClass() != obj.getClass()) + return false; + RabbitMq other = (RabbitMq) obj; + if (!Secret.toString(password).equals(other.getPassword())) + { + return false; + } + if (queue == null) + { + if (other.queue != null) + return false; + } + else if (!queue.equals(other.queue)) + { + return false; + } + if (username == null) + { + if (other.username != null) + return false; + } + else if (!username.equals(other.username)) + { + return false; + } + return true; + } + + @Override + public int hashCode() + { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((queue == null) ? 0 : queue.hashCode()); + result = prime * result + ((username == null) ? 0 : username.hashCode()); + result = prime * result + Secret.toString(password).hashCode(); + return result; } @Override diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Redis.java b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java index 2f3cd4f7..2992564d 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/Redis.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java @@ -45,11 +45,39 @@ public void setPassword(String password) } @Override - protected boolean shouldRefreshInstance() + public boolean equals(Object obj) { - return super.shouldRefreshInstance() || - !instance.getPassword().equals(Secret.toString(password))|| - !StringUtils.equals(instance.getKey(), key); + if (this == obj) + return true; + if (!super.equals(obj)) + return false; + if (getClass() != obj.getClass()) + return false; + Redis other = (Redis) obj; + if (!Secret.toString(password).equals(other.getPassword())) + { + return false; + } + if (key == null) + { + if (other.key != null) + return false; + } + else if (!key.equals(other.key)) + { + return false; + } + return true; + } + + @Override + public int hashCode() + { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((key == null) ? 0 : key.hashCode()); + result = prime * result + Secret.toString(password).hashCode(); + return result; } diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java index f69fdf3f..a258da06 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java @@ -16,8 +16,7 @@ public class Syslog extends LogstashIndexer @DataBoundConstructor public Syslog() - { - } + {} public MessageFormat getMessageFormat() { @@ -42,10 +41,27 @@ public void setSyslogProtocol(SyslogProtocol syslogProtocol) } @Override - protected boolean shouldRefreshInstance() + public int hashCode() + { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((messageFormat == null) ? 0 : messageFormat.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { - return super.shouldRefreshInstance() || - !instance.getMessageFormat().equals(messageFormat); + if (this == obj) + return true; + if (!super.equals(obj)) + return false; + if (getClass() != obj.getClass()) + return false; + Syslog other = (Syslog)obj; + if (messageFormat != other.messageFormat) + return false; + return true; } @Override diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java index 2f0c6b1a..91f9fbf0 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -8,16 +8,14 @@ import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; -import jenkins.plugins.logstash.persistence.ElasticSearchDao; - public class ElasticSearchTest { @Rule public JenkinsRule j = new JenkinsRule(); - private ElasticSearchDao dao; private ElasticSearch indexer; + private ElasticSearch indexer2; @Before public void setup() @@ -28,35 +26,40 @@ public void setup() indexer.setPassword("password"); indexer.setUsername("user"); indexer.setKey("key"); - dao = indexer.getInstance(); - } + + indexer2 = new ElasticSearch(); + indexer2.setHost("http://localhost"); + indexer2.setPort(4567); + indexer2.setPassword("password"); + indexer2.setUsername("user"); + indexer2.setKey("key"); +} @Test - public void test_noChangeReturnsSameInstance() + public void test_sameSettingsAreEqual() { - assertThat(indexer.shouldRefreshInstance(), is(false)); - assertThat(indexer.getInstance(),is(dao)); + assertThat(indexer.equals(indexer2), is(true)); } @Test - public void test_passwordChangeLeadsToNewInstance() + public void test_passwordChangeIsNotEqual() { indexer.setPassword("newPassword"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_usernameChangeLeadsToNewInstance() + public void test_usernameChangeIsNotEqual() { indexer.setUsername("newUser"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_KeyChangeLeadsToNewInstance() + public void test_KeyChangeIsNotEqual() { indexer.setKey("newKey"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } } diff --git a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java index 133fb43b..a1e3fb51 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java @@ -6,34 +6,39 @@ import org.junit.Before; import org.junit.Test; -import com.cloudbees.syslog.MessageFormat; - import jenkins.plugins.logstash.persistence.MemoryDao; public class LogstashIndexerTest { private LogstashIndexerForTest indexer; + private LogstashIndexerForTest indexer2; @Before public void setup() { indexer = new LogstashIndexerForTest("localhost", 4567); - indexer.getInstance(); + indexer2 = new LogstashIndexerForTest("localhost", 4567); + } + + @Test + public void test_sameSettingsAreEqual() + { + assertThat(indexer.equals(indexer2), is(true)); } @Test - public void test_HostChangeLeadsToNewInstance() + public void test_HostChangeIsNotEqual() { indexer.setHost("remoteHost"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_PortChangeLeadsToNewInstance() + public void test_PortChangeIsNotEqual() { indexer.setPort(7654); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } public static class LogstashIndexerForTest extends LogstashIndexer diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java index f398d107..f29acfae 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java @@ -8,8 +8,6 @@ import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; -import jenkins.plugins.logstash.persistence.RabbitMqDao; - public class RabbitMqTest { @@ -17,7 +15,7 @@ public class RabbitMqTest public JenkinsRule j = new JenkinsRule(); private RabbitMq indexer; - private RabbitMqDao dao; + private RabbitMq indexer2; @Before public void setup() @@ -28,35 +26,40 @@ public void setup() indexer.setPassword("password"); indexer.setUsername("user"); indexer.setQueue("queue"); - dao = indexer.getInstance(); + + indexer2 = new RabbitMq(); + indexer2.setHost("localhost"); + indexer2.setPort(4567); + indexer2.setPassword("password"); + indexer2.setUsername("user"); + indexer2.setQueue("queue"); } @Test - public void test_noChangeReturnsSameInstance() + public void test_sameSettingsAreEqual() { - assertThat(indexer.shouldRefreshInstance(), is(false)); - assertThat(indexer.getInstance(),is(dao)); + assertThat(indexer.equals(indexer2), is(true)); } @Test - public void test_passwordChangeLeadsToNewInstance() + public void test_passwordChangeIsNotEqual() { indexer.setPassword("newPassword"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_usernameChangeLeadsToNewInstance() + public void test_usernameChangeIsNotEqual() { indexer.setUsername("newUser"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_QueueChangeLeadsToNewInstance() + public void test_QueueChangeIsNotEqual() { indexer.setQueue("newQueue"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } } diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java index b938e405..5329c5d3 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java @@ -8,8 +8,6 @@ import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; -import jenkins.plugins.logstash.persistence.RedisDao; - public class RedisTest { @@ -17,7 +15,7 @@ public class RedisTest public JenkinsRule j = new JenkinsRule(); private Redis indexer; - private RedisDao dao; + private Redis indexer2; @Before public void setup() @@ -27,29 +25,32 @@ public void setup() indexer.setPort(4567); indexer.setKey("key"); indexer.setPassword("password"); - dao = indexer.getInstance(); - } + + indexer2 = new Redis(); + indexer2.setHost("localhost"); + indexer2.setPort(4567); + indexer2.setKey("key"); + indexer2.setPassword("password"); +} @Test - public void test_noChangeReturnsSameInstance() + public void test_sameSettingsAreEqual() { - assertThat(indexer.shouldRefreshInstance(), is(false)); - assertThat(indexer.getInstance(),is(dao)); + assertThat(indexer.equals(indexer2), is(true)); } - @Test - public void test_passwordChangeLeadsToNewInstance() + public void test_passwordChangeIsNotEqual() { indexer.setPassword("newPassword"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } @Test - public void test_KeyChangeLeadsToNewInstance() + public void test_KeyChangeIsNotEqual() { indexer.setKey("newKey"); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } diff --git a/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java index 2d77b342..45f9b21a 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java @@ -8,13 +8,11 @@ import com.cloudbees.syslog.MessageFormat; -import jenkins.plugins.logstash.persistence.SyslogDao; - public class SyslogTest { private Syslog indexer; - private SyslogDao dao; + private Syslog indexer2; @Before public void setup() @@ -23,21 +21,24 @@ public void setup() indexer.setHost("localhost"); indexer.setPort(4567); indexer.setMessageFormat(MessageFormat.RFC_3164); - dao = indexer.getInstance(); - } + + indexer2 = new Syslog(); + indexer2.setHost("localhost"); + indexer2.setPort(4567); + indexer2.setMessageFormat(MessageFormat.RFC_3164); +} @Test - public void test_noChangeReturnsSameInstance() + public void test_sameSettingsAreEqual() { - assertThat(indexer.shouldRefreshInstance(), is(false)); - assertThat(indexer.getInstance(),is(dao)); + assertThat(indexer.equals(indexer2), is(true)); } @Test - public void test_messageFormatChangeLeadsToNewInstance() + public void test_messageFormatChangeIsNotEqual() { indexer.setMessageFormat(MessageFormat.RFC_5424); - assertThat(indexer.shouldRefreshInstance(), is(true)); + assertThat(indexer.equals(indexer2), is(false)); } } From d602a6ef45a3628463eb16fd8875a6e2d97dd515 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 3 Jan 2018 13:50:23 +0100 Subject: [PATCH 09/17] incorporate feedback rename junit test methods synchronize getting the dao instance to avoid problems. deprecate enums --- pom.xml | 2 +- .../logstash/configuration/LogstashIndexer.java | 2 +- .../logstash/persistence/LogstashIndexerDao.java | 4 +++- src/main/resources/index.jelly | 3 +++ .../logstash/LogstashConfigurationMigrationTest.java | 10 +++++----- .../plugins/logstash/LogstashConfigurationTest.java | 10 +++++----- .../plugins/logstash/LogstashIntegrationTest.java | 8 ++++---- .../logstash/configuration/ElasticSearchTest.java | 8 ++++---- .../logstash/configuration/LogstashIndexerTest.java | 4 ++-- .../plugins/logstash/configuration/RabbitMqTest.java | 8 ++++---- .../plugins/logstash/configuration/RedisTest.java | 6 +++--- .../plugins/logstash/configuration/SyslogTest.java | 4 ++-- .../plugins/logstash/persistence/SyslogDaoTest.java | 1 - 13 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 src/main/resources/index.jelly diff --git a/pom.xml b/pom.xml index ae28f223..a7be2ab3 100644 --- a/pom.xml +++ b/pom.xml @@ -52,7 +52,7 @@ logstash hpi - 1.4.0-SNAPSHOT + 2.0-SNAPSHOT Logstash A Logstash agent to send Jenkins logs to a Logstash indexer. https://wiki.jenkins-ci.org/display/JENKINS/Logstash+Plugin diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index dcd43e89..a9b65971 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -88,7 +88,7 @@ public void setPort(int port) * @return {@link AbstractLogstashIndexerDao} instance */ @Nonnull - public final T getInstance() + public synchronized final T getInstance() { if (shouldRefreshInstance()) { diff --git a/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java index ed7e0374..0229ad88 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/LogstashIndexerDao.java @@ -37,13 +37,15 @@ * @since 1.0.0 */ public interface LogstashIndexerDao { - public static enum IndexerType { + @Deprecated + static enum IndexerType { REDIS, RABBIT_MQ, ELASTICSEARCH, SYSLOG } + @Deprecated static enum SyslogFormat { RFC5424, RFC3164 diff --git a/src/main/resources/index.jelly b/src/main/resources/index.jelly new file mode 100644 index 00000000..565ea906 --- /dev/null +++ b/src/main/resources/index.jelly @@ -0,0 +1,3 @@ +
+ Adds the possibility to push builds logs and build data to a Logstash indexer such as Redis, RabbitMQ, Elastic Search or to Syslog. +
diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java index 7f8a8f0c..d5e2bf0d 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java @@ -59,7 +59,7 @@ public void setup() } @Test - public void test_redisMigration() + public void redisMigration() { when(descriptor.getType()).thenReturn(IndexerType.REDIS); configuration.migrateData(); @@ -73,7 +73,7 @@ public void test_redisMigration() } @Test - public void test_syslogMigrationRFC3164() + public void syslogMigrationRFC3164() { when(descriptor.getType()).thenReturn(IndexerType.SYSLOG); when(descriptor.getSyslogFormat()).thenReturn(SyslogFormat.RFC3164); @@ -87,7 +87,7 @@ public void test_syslogMigrationRFC3164() } @Test - public void test_syslogMigrationRFC5424() + public void syslogMigrationRFC5424() { when(descriptor.getType()).thenReturn(IndexerType.SYSLOG); when(descriptor.getSyslogFormat()).thenReturn(SyslogFormat.RFC5424); @@ -101,7 +101,7 @@ public void test_syslogMigrationRFC5424() } @Test - public void test_elasticSearchMigration() + public void elasticSearchMigration() { when(descriptor.getType()).thenReturn(IndexerType.ELASTICSEARCH); configuration.migrateData(); @@ -116,7 +116,7 @@ public void test_elasticSearchMigration() } @Test - public void test_rabbitMqMigration() + public void rabbitMqMigration() { when(descriptor.getType()).thenReturn(IndexerType.RABBIT_MQ); configuration.migrateData(); diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java index cb73c3de..70dd3912 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java @@ -18,7 +18,7 @@ public class LogstashConfigurationTest extends LogstashConfigurationTestBase { @Test - public void test_unconfiguredWillReturnNull() + public void unconfiguredWillReturnNull() { LogstashConfigurationTestBase.configFile = new File("src/test/resources/notExisting.xml"); LogstashConfiguration configuration = new LogstashConfigurationForTest(); @@ -26,7 +26,7 @@ public void test_unconfiguredWillReturnNull() } @Test - public void test_elasticSearchIsProperlyConfigured() + public void elasticSearchIsProperlyConfigured() { LogstashConfigurationTestBase.configFile = new File("src/test/resources/elasticSearch.xml"); LogstashConfiguration configuration = new LogstashConfigurationForTest(); @@ -34,7 +34,7 @@ public void test_elasticSearchIsProperlyConfigured() } @Test - public void test_rabbitMqIsProperlyConfigured() + public void rabbitMqIsProperlyConfigured() { LogstashConfigurationTestBase.configFile = new File("src/test/resources/rabbitmq.xml"); LogstashConfiguration configuration = new LogstashConfigurationForTest(); @@ -42,7 +42,7 @@ public void test_rabbitMqIsProperlyConfigured() } @Test - public void test_redisIsProperlyConfigured() + public void redisIsProperlyConfigured() { LogstashConfigurationTestBase.configFile = new File("src/test/resources/redis.xml"); LogstashConfiguration configuration = new LogstashConfigurationForTest(); @@ -50,7 +50,7 @@ public void test_redisIsProperlyConfigured() } @Test - public void test_syslogIsProperlyConfigured() + public void syslogIsProperlyConfigured() { LogstashConfigurationTestBase.configFile = new File("src/test/resources/syslog.xml"); LogstashConfiguration configuration = new LogstashConfigurationForTest(); diff --git a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java index 7de58a59..bb719a0a 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java @@ -57,7 +57,7 @@ public void setup() throws Exception } @Test - public void test_buildWrapperOnMaster() throws Exception + public void buildWrapperOnMaster() throws Exception { project.getBuildWrappersList().add(new LogstashBuildWrapper()); QueueTaskFuture f = project.scheduleBuild2(0); @@ -74,7 +74,7 @@ public void test_buildWrapperOnMaster() throws Exception } @Test - public void test_buildWrapperOnSlave() throws Exception + public void buildWrapperOnSlave() throws Exception { project.getBuildWrappersList().add(new LogstashBuildWrapper()); project.setAssignedNode(slave); @@ -93,7 +93,7 @@ public void test_buildWrapperOnSlave() throws Exception } @Test - public void test_buildNotifierOnMaster() throws Exception + public void buildNotifierOnMaster() throws Exception { project.getPublishersList().add(new LogstashNotifier(10, false)); QueueTaskFuture f = project.scheduleBuild2(0); @@ -108,7 +108,7 @@ public void test_buildNotifierOnMaster() throws Exception } @Test - public void test_buildNotifierOnSlave() throws Exception + public void buildNotifierOnSlave() throws Exception { project.getPublishersList().add(new LogstashNotifier(10, false)); project.setAssignedNode(slave); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java index 2f0c6b1a..62e0eed8 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -32,28 +32,28 @@ public void setup() } @Test - public void test_noChangeReturnsSameInstance() + public void noChangeReturnsSameInstance() { assertThat(indexer.shouldRefreshInstance(), is(false)); assertThat(indexer.getInstance(),is(dao)); } @Test - public void test_passwordChangeLeadsToNewInstance() + public void passwordChangeLeadsToNewInstance() { indexer.setPassword("newPassword"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_usernameChangeLeadsToNewInstance() + public void usernameChangeLeadsToNewInstance() { indexer.setUsername("newUser"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_KeyChangeLeadsToNewInstance() + public void keyChangeLeadsToNewInstance() { indexer.setKey("newKey"); assertThat(indexer.shouldRefreshInstance(), is(true)); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java index 133fb43b..693b77c1 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java @@ -23,14 +23,14 @@ public void setup() } @Test - public void test_HostChangeLeadsToNewInstance() + public void hostChangeLeadsToNewInstance() { indexer.setHost("remoteHost"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_PortChangeLeadsToNewInstance() + public void portChangeLeadsToNewInstance() { indexer.setPort(7654); assertThat(indexer.shouldRefreshInstance(), is(true)); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java index f398d107..2937d347 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/RabbitMqTest.java @@ -32,28 +32,28 @@ public void setup() } @Test - public void test_noChangeReturnsSameInstance() + public void noChangeReturnsSameInstance() { assertThat(indexer.shouldRefreshInstance(), is(false)); assertThat(indexer.getInstance(),is(dao)); } @Test - public void test_passwordChangeLeadsToNewInstance() + public void passwordChangeLeadsToNewInstance() { indexer.setPassword("newPassword"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_usernameChangeLeadsToNewInstance() + public void usernameChangeLeadsToNewInstance() { indexer.setUsername("newUser"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_QueueChangeLeadsToNewInstance() + public void queueChangeLeadsToNewInstance() { indexer.setQueue("newQueue"); assertThat(indexer.shouldRefreshInstance(), is(true)); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java index b938e405..1194e1fa 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/RedisTest.java @@ -31,7 +31,7 @@ public void setup() } @Test - public void test_noChangeReturnsSameInstance() + public void noChangeReturnsSameInstance() { assertThat(indexer.shouldRefreshInstance(), is(false)); assertThat(indexer.getInstance(),is(dao)); @@ -39,14 +39,14 @@ public void test_noChangeReturnsSameInstance() @Test - public void test_passwordChangeLeadsToNewInstance() + public void passwordChangeLeadsToNewInstance() { indexer.setPassword("newPassword"); assertThat(indexer.shouldRefreshInstance(), is(true)); } @Test - public void test_KeyChangeLeadsToNewInstance() + public void keyChangeLeadsToNewInstance() { indexer.setKey("newKey"); assertThat(indexer.shouldRefreshInstance(), is(true)); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java index 2d77b342..01655ec0 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/SyslogTest.java @@ -27,14 +27,14 @@ public void setup() } @Test - public void test_noChangeReturnsSameInstance() + public void noChangeReturnsSameInstance() { assertThat(indexer.shouldRefreshInstance(), is(false)); assertThat(indexer.getInstance(),is(dao)); } @Test - public void test_messageFormatChangeLeadsToNewInstance() + public void messageFormatChangeLeadsToNewInstance() { indexer.setMessageFormat(MessageFormat.RFC_5424); assertThat(indexer.shouldRefreshInstance(), is(true)); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java index f37b44c3..a23a0890 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTest.java @@ -21,7 +21,6 @@ public class SyslogDaoTest { String host = "localhost"; String appname = "jenkins:"; int port = 514; - UdpSyslogMessageSender udpSyslogMessageSender = new UdpSyslogMessageSender(); @Mock UdpSyslogMessageSender mockUdpSyslogMessageSender; @Before From 61164106f20c36e70b5cbb2aa59439c16a3b6a64 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 3 Jan 2018 15:01:54 +0100 Subject: [PATCH 10/17] adjust link to plugin page --- pom.xml | 12 +++++++++++- .../logstash/LogstashInstallation/global.jelly | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index a7be2ab3..ae2d190a 100644 --- a/pom.xml +++ b/pom.xml @@ -55,7 +55,7 @@ 2.0-SNAPSHOT Logstash A Logstash agent to send Jenkins logs to a Logstash indexer. - https://wiki.jenkins-ci.org/display/JENKINS/Logstash+Plugin + https://plugins.jenkins.io/logstash @@ -143,6 +143,16 @@ + + + org.jenkins-ci.tools + maven-hpi-plugin + true + + 2.0 + + + org.apache.maven.plugins diff --git a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly index c76deafe..f0055c8a 100644 --- a/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly +++ b/src/main/resources/jenkins/plugins/logstash/LogstashInstallation/global.jelly @@ -2,7 +2,7 @@ -

Logstash configuration has moved to the Global Configuration

+ Logstash configuration has moved to the Global Configuration From ac70407711e8543981c7f5d41e11265325e8af7c Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 3 Jan 2018 21:00:58 +0100 Subject: [PATCH 11/17] simplify equals --- .../java/jenkins/plugins/logstash/LogstashConfiguration.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 4579df82..86a24090 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -1,6 +1,7 @@ package jenkins.plugins.logstash; import java.util.List; +import java.util.Objects; import java.util.logging.Level; import java.util.logging.Logger; @@ -151,8 +152,7 @@ public boolean configure(StaplerRequest staplerRequest, JSONObject json) throws { // when we bind the stapler request we get a new instance of logstashIndexer staplerRequest.bindJSON(this, json); - - if (logstashIndexer != null && !logstashIndexer.equals(activeIndexer)) + if (!Objects.equals(logstashIndexer, activeIndexer)) { activeIndexer = logstashIndexer; } From 87c93cb04df2c7bfb94949c26323772b10c8c912 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 4 Jan 2018 01:05:29 +0100 Subject: [PATCH 12/17] host based indexers vs uri introduce an abstract host based indexer that serves as base for most indexers. Elastic search uses a uri approach where we need scheme, host, port and url path, so we allow to configure a uri instead. --- .../logstash/LogstashConfiguration.java | 26 +++-- .../logstash/configuration/ElasticSearch.java | 71 +++++++----- .../HostBasedLogstashIndexer.java | 83 ++++++++++++++ .../configuration/HostConfiguration.java | 66 +++++++++++ .../configuration/LogstashIndexer.java | 75 ------------- .../logstash/configuration/RabbitMq.java | 10 +- .../plugins/logstash/configuration/Redis.java | 4 +- .../logstash/configuration/Syslog.java | 4 +- .../AbstractLogstashIndexerDao.java | 25 ----- .../persistence/ElasticSearchDao.java | 75 ++++++++----- .../HostBasedLogstashIndexerDao.java | 104 ++++++++++++++++++ .../logstash/persistence/RabbitMqDao.java | 2 +- .../logstash/persistence/RedisDao.java | 2 +- .../logstash/persistence/SyslogDao.java | 2 +- ...{configure-advanced.jelly => config.jelly} | 6 +- .../ElasticSearch/help-host.html | 4 - .../configuration/ElasticSearch/help-key.html | 3 - .../configuration/ElasticSearch/help-uri.html | 3 + .../config.jelly | 0 .../help-host.html | 0 .../help-port.html | 0 .../LogstashConfigurationMigrationTest.java | 11 +- .../logstash/LogstashIntegrationTest.java | 2 +- .../configuration/ElasticSearchTest.java | 65 +++++++++-- ...java => HostBasedLogstashIndexerTest.java} | 6 +- .../AbstractLogstashIndexerDaoTest.java | 8 +- .../persistence/ElasticSearchDaoTest.java | 82 +++++--------- .../logstash/persistence/MemoryDao.java | 10 +- .../logstash/persistence/SyslogDaoTestIT.java | 4 + src/test/resources/elasticSearch.xml | 4 +- 30 files changed, 498 insertions(+), 259 deletions(-) create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer.java create mode 100644 src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java create mode 100644 src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java rename src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/{configure-advanced.jelly => config.jelly} (82%) delete mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html delete mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html create mode 100644 src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-uri.html rename src/main/resources/jenkins/plugins/logstash/configuration/{LogstashIndexer => HostBasedLogstashIndexer}/config.jelly (100%) rename src/main/resources/jenkins/plugins/logstash/configuration/{LogstashIndexer => HostBasedLogstashIndexer}/help-host.html (100%) rename src/main/resources/jenkins/plugins/logstash/configuration/{LogstashIndexer => HostBasedLogstashIndexer}/help-port.html (100%) rename src/test/java/jenkins/plugins/logstash/configuration/{LogstashIndexerTest.java => HostBasedLogstashIndexerTest.java} (86%) diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 86a24090..0807a6cf 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -1,5 +1,7 @@ package jenkins.plugins.logstash; +import java.net.URI; +import java.net.URISyntaxException; import java.util.List; import java.util.Objects; import java.util.logging.Level; @@ -11,6 +13,7 @@ import com.cloudbees.syslog.MessageFormat; +import org.apache.http.client.utils.URIBuilder; import hudson.Extension; import hudson.init.InitMilestone; import hudson.init.Initializer; @@ -76,6 +79,7 @@ public List getIndexerTypes() return LogstashIndexer.all(); } + @SuppressWarnings("deprecation") @Initializer(after = InitMilestone.JOB_LOADED) public void migrateData() { @@ -98,13 +102,21 @@ public void migrateData() break; case ELASTICSEARCH: LOGGER.log(Level.INFO, "Migrating logstash configuration for Elastic Search"); - ElasticSearch es = new ElasticSearch(); - es.setHost(descriptor.getHost()); - es.setPort(descriptor.getPort()); - es.setKey(descriptor.getKey()); - es.setUsername(descriptor.getUsername()); - es.setPassword(descriptor.getPassword()); - logstashIndexer = es; + URI uri; + try + { + uri = (new URIBuilder(descriptor.getHost())) + .setPort(descriptor.getPort()) + .setPath("/" + descriptor.getKey()).build(); + ElasticSearch es = new ElasticSearch(uri.toString()); + es.setUsername(descriptor.getUsername()); + es.setPassword(descriptor.getPassword()); + logstashIndexer = es; + } + catch (URISyntaxException e) + { + LOGGER.log(Level.INFO, "Migrating logstash configuration for Elastic Search failed: " + e.toString()); + } break; case RABBIT_MQ: LOGGER.log(Level.INFO, "Migrating logstash configuration for RabbitMQ"); diff --git a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java index 2add16a7..502cf1a8 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java @@ -1,7 +1,8 @@ package jenkins.plugins.logstash.configuration; import java.net.MalformedURLException; -import java.net.URL; +import java.net.URI; +import java.net.URISyntaxException; import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.DataBoundConstructor; @@ -16,24 +17,27 @@ public class ElasticSearch extends LogstashIndexer { - protected String key; - protected String username; - protected Secret password; + private String username; + private Secret password; + private URI uri; @DataBoundConstructor - public ElasticSearch() + public ElasticSearch(String uri) throws URISyntaxException { + this.uri = new URI(uri); + validateUri(this.uri); } - public String getKey() + public URI getUri() { - return key; + return uri; } - @DataBoundSetter - public void setKey(String key) + public void setUri(String value) throws URISyntaxException { - this.key = key; + URI uri = new URI(value); + validateUri(uri); + this.uri = uri; } public String getUsername() @@ -61,10 +65,10 @@ public void setPassword(String password) @Override public boolean equals(Object obj) { + if (obj == null) + return false; if (this == obj) return true; - if (!super.equals(obj)) - return false; if (getClass() != obj.getClass()) return false; ElasticSearch other = (ElasticSearch) obj; @@ -72,12 +76,12 @@ public boolean equals(Object obj) { return false; } - if (key == null) + if (uri == null) { - if (other.key != null) + if (other.uri != null) return false; } - else if (!key.equals(other.key)) + else if (!uri.equals(other.uri)) { return false; } @@ -98,7 +102,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); - result = prime * result + ((key == null) ? 0 : key.hashCode()); + result = prime * result + ((uri == null) ? 0 : uri.hashCode()); result = prime * result + ((username == null) ? 0 : username.hashCode()); result = prime * result + Secret.toString(password).hashCode(); return result; @@ -107,7 +111,7 @@ public int hashCode() @Override public ElasticSearchDao createIndexerInstance() { - return new ElasticSearchDao(host, port, key, username, Secret.toString(password)); + return new ElasticSearchDao(uri, username, Secret.toString(password)); } @Extension @@ -125,8 +129,7 @@ public int getDefaultPort() return 9300; } - @Override - public FormValidation doCheckHost(@QueryParameter("value") String value) + public FormValidation doCheckUri(@QueryParameter("value") String value) { if (StringUtils.isBlank(value)) { @@ -134,24 +137,40 @@ public FormValidation doCheckHost(@QueryParameter("value") String value) } try { - new URL(value); + URI uri = new URI(value); + validateUri(uri); } - catch (MalformedURLException e) + catch (URISyntaxException | IllegalArgumentException e) { return FormValidation.error(e.getMessage()); } return FormValidation.ok(); } + } - public FormValidation doCheckKey(@QueryParameter("value") String value) - { - if (StringUtils.isBlank(value)) + /** + * Validates that the given uri has a scheme, a port and a path which is not empty or just "/" + * + * @param uri + * @throws IllegalArgumentException when one of scheme, port or path + */ + public static void validateUri(URI uri) throws IllegalArgumentException + { + try { - return FormValidation.error(Messages.ValueIsRequired()); + uri.toURL(); + } + catch (MalformedURLException e) + { + throw new IllegalArgumentException(e.getMessage()); } - return FormValidation.ok(); + if(uri.getPort() == -1) { + throw new IllegalArgumentException("Please specify a port."); } + if(StringUtils.isBlank(uri.getPath()) || uri.getPath().trim().matches("^\\/+$")) { + throw new IllegalArgumentException("Please specify an elastic search key."); + } } } diff --git a/src/main/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer.java new file mode 100644 index 00000000..055551a6 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer.java @@ -0,0 +1,83 @@ +package jenkins.plugins.logstash.configuration; + +import org.kohsuke.stapler.DataBoundSetter; + +import jenkins.plugins.logstash.persistence.AbstractLogstashIndexerDao; + +public abstract class HostBasedLogstashIndexer extends LogstashIndexer +{ + private String host; + private int port; + + /** + * Returns the host for connecting to the indexer. + * + * @return Host of the indexer + */ + public String getHost() + { + return host; + } + + /** + * Sets the host for connecting to the indexer. + * + * @param host + * host to connect to. + */ + @DataBoundSetter + public void setHost(String host) + { + this.host = host; + } + + /** + * Returns the port for connecting to the indexer. + * + * @return Port of the indexer + */ + public int getPort() + { + return port; + } + + /** + * Sets the port used for connecting to the indexer + * + * @param port + * The port of the indexer + */ + @DataBoundSetter + public void setPort(int port) + { + this.port = port; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((host == null) ? 0 : host.hashCode()); + result = prime * result + port; + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + HostBasedLogstashIndexer other = (HostBasedLogstashIndexer) obj; + if (host == null) { + if (other.host != null) + return false; + } else if (!host.equals(other.host)) + return false; + if (port != other.port) + return false; + return true; + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java b/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java new file mode 100644 index 00000000..907030f3 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java @@ -0,0 +1,66 @@ +package jenkins.plugins.logstash.configuration; + +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; + +public class HostConfiguration +{ + + protected String host; + protected int port; + + @DataBoundConstructor + public HostConfiguration() + { + } + + public String getHost() + { + return host; + } + + @DataBoundSetter + public void setHost(String host) + { + this.host = host; + } + + public int getPort() + { + return port; + } + + @DataBoundSetter + public void setPort(int port) + { + this.port = port; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((host == null) ? 0 : host.hashCode()); + result = prime * result + port; + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + HostConfiguration other = (HostConfiguration) obj; + if (host == null) { + if (other.host != null) + return false; + } else if (!host.equals(other.host)) + return false; + if (port != other.port) + return false; + return true; + } +} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index 4e4f81f1..94acc951 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -30,55 +30,8 @@ public abstract class LogstashIndexer extends AbstractDescribableImpl> implements ExtensionPoint, ReconfigurableDescribable> { - protected String host; - protected int port; - protected transient T instance; - /** - * Returns the host for connecting to the indexer. - * - * @return Host of the indexer - */ - public String getHost() - { - return host; - } - - /** - * Sets the host for connecting to the indexer. - * - * @param host - * host to connect to. - */ - @DataBoundSetter - public void setHost(String host) - { - this.host = host; - } - - /** - * Returns the port for connecting to the indexer. - * - * @return Port of the indexer - */ - public int getPort() - { - return port; - } - - /** - * Sets the port used for connecting to the indexer - * - * @param port - * The port of the indexer - */ - @DataBoundSetter - public void setPort(int port) - { - this.port = port; - } - /** * Gets the instance of the actual {@link AbstractLogstashIndexerDao} that is represented by this * configuration. @@ -103,34 +56,6 @@ public synchronized final T getInstance() */ protected abstract T createIndexerInstance(); - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((host == null) ? 0 : host.hashCode()); - result = prime * result + port; - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - LogstashIndexer other = (LogstashIndexer) obj; - if (host == null) { - if (other.host != null) - return false; - } else if (!host.equals(other.host)) - return false; - if (port != other.port) - return false; - return true; - } - @SuppressWarnings("unchecked") public static DescriptorExtensionList, Descriptor>> all() diff --git a/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java index 48ed671f..b581ba84 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/RabbitMq.java @@ -11,12 +11,12 @@ import jenkins.plugins.logstash.Messages; import jenkins.plugins.logstash.persistence.RabbitMqDao; -public class RabbitMq extends LogstashIndexer +public class RabbitMq extends HostBasedLogstashIndexer { - protected String queue; - protected String username; - protected Secret password; + private String queue; + private String username; + private Secret password; @DataBoundConstructor public RabbitMq() @@ -105,7 +105,7 @@ public int hashCode() @Override public RabbitMqDao createIndexerInstance() { - return new RabbitMqDao(host, port, queue, username, Secret.toString(password)); + return new RabbitMqDao(getHost(), getPort(), queue, username, Secret.toString(password)); } @Extension diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Redis.java b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java index 2992564d..a1f22afd 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/Redis.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/Redis.java @@ -11,7 +11,7 @@ import jenkins.plugins.logstash.Messages; import jenkins.plugins.logstash.persistence.RedisDao; -public class Redis extends LogstashIndexer +public class Redis extends HostBasedLogstashIndexer { protected String key; @@ -84,7 +84,7 @@ public int hashCode() @Override public RedisDao createIndexerInstance() { - return new RedisDao(host, port, key, Secret.toString(password)); + return new RedisDao(getHost(), getPort(), key, Secret.toString(password)); } @Extension diff --git a/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java index 35d167e4..748ea44f 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/Syslog.java @@ -9,7 +9,7 @@ import jenkins.plugins.logstash.persistence.LogstashIndexerDao.SyslogProtocol; import jenkins.plugins.logstash.persistence.SyslogDao; -public class Syslog extends LogstashIndexer +public class Syslog extends HostBasedLogstashIndexer { private MessageFormat messageFormat; private SyslogProtocol syslogProtocol; @@ -70,7 +70,7 @@ public boolean equals(Object obj) @Override public SyslogDao createIndexerInstance() { - SyslogDao syslogDao = new SyslogDao(host, port); + SyslogDao syslogDao = new SyslogDao(getHost(), getPort()); syslogDao.setMessageFormat(messageFormat); return syslogDao; } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java index 088b0200..9b6f48a2 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java @@ -39,18 +39,8 @@ * @since 1.0.0 */ public abstract class AbstractLogstashIndexerDao implements LogstashIndexerDao { - private final String host; - private final int port; private Charset charset; - public AbstractLogstashIndexerDao(String host, int port) { - this.host = host; - this.port = port; - if (StringUtils.isBlank(host)) { - throw new IllegalArgumentException("host name is required"); - } - } - /** * Sets the charset used to push data to the indexer * @@ -85,19 +75,4 @@ public JSONObject buildPayload(BuildData buildData, String jenkinsUrl, List successCodes = closedOpen(200,300); - private String key; private String username; private String password; //primary constructor used by indexer factory - public ElasticSearchDao(String host, int port, String key, String username, String password) { - this(null, host, port, key, username, password); + public ElasticSearchDao(URI uri, String username, String password) { + this(null, uri, username, password); } // Factored for unit testing - ElasticSearchDao(HttpClientBuilder factory, String host, int port, String key, String username, String password) { - super(host, port); - - this.key = key; - this.username = username; - this.password = password; + ElasticSearchDao(HttpClientBuilder factory, URI uri, String username, String password) { - if (StringUtils.isBlank(key)) { - throw new IllegalArgumentException("elastic index name is required"); + if (uri == null) + { + throw new IllegalArgumentException("uri field must not be empty"); } - try { - uri = new URIBuilder(host) - .setPort(port) - // Normalizer will remove extra starting slashes, but missing slash will cause annoying failures - .setPath("/" + key) - .build(); - } catch (URISyntaxException e) { - throw new IllegalArgumentException("Could not create uri", e); - } + this.uri = uri; + this.username = username; + this.password = password; - if(StringUtils.isBlank(uri.getScheme())) { - throw new IllegalArgumentException("host field must specify scheme, such as 'http://'"); - } + ElasticSearch.validateUri(uri); if (StringUtils.isNotBlank(username)) { auth = Base64.encodeBase64String((username + ":" + StringUtils.defaultString(password)).getBytes(StandardCharsets.UTF_8)); @@ -109,9 +99,19 @@ public URI getUri() return uri; } - public String getKey() + public String getHost() + { + return uri.getHost(); + } + + public String getScheme() + { + return uri.getScheme(); + } + + public int getPort() { - return key; + return uri.getPort(); } public String getUsername() @@ -124,6 +124,11 @@ public String getPassword() return password; } + public String getKey() + { + return uri.getPath(); + } + String getAuth() { return auth; @@ -192,4 +197,24 @@ private String getErrorMessage(CloseableHttpResponse response) { } } } + + public static void main(String[] args) throws URISyntaxException, MalformedURLException + { + URI uri = (new URIBuilder("localhost:8000")).setPath("/logstash").build(); + + //System.out.println(uri.toURL()); + System.out.println("host:" + uri.getHost()); + System.out.println("port:" + uri.getPort()); + System.out.println("path:" + uri.getPath()); + System.out.println("scheme:" + uri.getScheme()); + System.out.println("fragment:" + uri.getFragment()); + System.out.println("rawpath:" + uri.getRawPath()); + + } + + @Override + public String getDescription() + { + return uri.toString(); + } } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java new file mode 100644 index 00000000..6d11d0dc --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java @@ -0,0 +1,104 @@ +/* + * The MIT License + * + * Copyright 2014 Rusty Gerard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package jenkins.plugins.logstash.persistence; + +import java.nio.charset.Charset; +import java.util.Calendar; +import java.util.List; + +import org.apache.commons.lang.StringUtils; + +import net.sf.json.JSONObject; + +/** + * Abstract data access object for Logstash indexers. + * + * @author Rusty Gerard + * @since 1.0.0 + */ +public abstract class HostBasedLogstashIndexerDao extends AbstractLogstashIndexerDao { + private final String host; + private final int port; + private Charset charset; + + public HostBasedLogstashIndexerDao(String host, int port) { + this.host = host; + this.port = port; + if (StringUtils.isBlank(host)) { + throw new IllegalArgumentException("host name is required"); + } + } + + /** + * Sets the charset used to push data to the indexer + * + *@param charset The charset to push data + */ + @Override + public void setCharset(Charset charset) + { + this.charset = charset; + } + + /** + * Gets the configured charset used to push data to the indexer + * + * @return charste to push data + */ + @Override + public Charset getCharset() + { + return charset; + } + + @Override + public JSONObject buildPayload(BuildData buildData, String jenkinsUrl, List logLines) { + JSONObject payload = new JSONObject(); + payload.put("data", buildData.toJson()); + payload.put("message", logLines); + payload.put("source", "jenkins"); + payload.put("source_host", jenkinsUrl); + payload.put("@buildTimestamp", buildData.getTimestamp()); + payload.put("@timestamp", BuildData.getDateFormatter().format(Calendar.getInstance().getTime())); + payload.put("@version", 1); + + return payload; + } + + public String getHost() + { + return host; + } + + public int getPort() + { + return port; + } + + @Override + public String getDescription() { + return this.host + ":" + this.port; + } +} diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java index b374a744..61d9fae5 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java @@ -38,7 +38,7 @@ * @author Rusty Gerard * @since 1.0.0 */ -public class RabbitMqDao extends AbstractLogstashIndexerDao { +public class RabbitMqDao extends HostBasedLogstashIndexerDao { private final ConnectionFactory pool; private String queue; diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java index 9b5677eb..e0fbb7d7 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java @@ -40,7 +40,7 @@ * @author Rusty Gerard * @since 1.0.0 */ -public class RedisDao extends AbstractLogstashIndexerDao { +public class RedisDao extends HostBasedLogstashIndexerDao { private final JedisPool pool; private String password; diff --git a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java index ba1bb043..70c5b105 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java @@ -7,7 +7,7 @@ import com.cloudbees.syslog.Severity; import com.cloudbees.syslog.sender.UdpSyslogMessageSender; -public class SyslogDao extends AbstractLogstashIndexerDao { +public class SyslogDao extends HostBasedLogstashIndexerDao { private MessageFormat messageFormat = MessageFormat.RFC_3164; private final UdpSyslogMessageSender messageSender; diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly similarity index 82% rename from src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly rename to src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly index 4113bc28..a01fab63 100644 --- a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/configure-advanced.jelly +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly @@ -1,12 +1,12 @@ + + + - - - \ No newline at end of file diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html deleted file mode 100644 index fdfed43b..00000000 --- a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-host.html +++ /dev/null @@ -1,4 +0,0 @@ -
-

The host name or IP address of the indexer to send log data to.
- Also specify scheme. Example: "https://myserver".

-
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html deleted file mode 100644 index 3bb63857..00000000 --- a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-key.html +++ /dev/null @@ -1,3 +0,0 @@ -
- The name and type path. Example: "/indexName/type" -
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-uri.html b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-uri.html new file mode 100644 index 00000000..9c7fab04 --- /dev/null +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/help-uri.html @@ -0,0 +1,3 @@ +
+ The full url to the elastic search index including scheme, port and key (<scheme>://<host>:<port>/<key>) +
diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/config.jelly similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/config.jelly rename to src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/config.jelly diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html b/src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/help-host.html similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-host.html rename to src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/help-host.html diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-port.html b/src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/help-port.html similarity index 100% rename from src/main/resources/jenkins/plugins/logstash/configuration/LogstashIndexer/help-port.html rename to src/main/resources/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexer/help-port.html diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java index d5e2bf0d..9cd5cbd3 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java @@ -7,7 +7,10 @@ import static org.powermock.api.mockito.PowerMockito.when; import java.io.File; +import java.net.URI; +import java.net.URISyntaxException; +import org.apache.http.client.utils.URIBuilder; import org.hamcrest.core.IsInstanceOf; import org.junit.Before; import org.junit.Rule; @@ -101,16 +104,16 @@ public void syslogMigrationRFC5424() } @Test - public void elasticSearchMigration() + public void elasticSearchMigration() throws URISyntaxException { when(descriptor.getType()).thenReturn(IndexerType.ELASTICSEARCH); + when(descriptor.getHost()).thenReturn("http://localhost"); configuration.migrateData(); LogstashIndexer indexer = configuration.getLogstashIndexer(); assertThat(indexer, IsInstanceOf.instanceOf(ElasticSearch.class)); ElasticSearch es = (ElasticSearch) indexer; - assertThat(es.getHost(),equalTo("localhost")); - assertThat(es.getPort(),is(4567)); - assertThat(es.getKey(), equalTo("logstash")); + URI uri = (new URIBuilder("http://localhost")).setPort(4567).setPath("/logstash").build(); + assertThat(es.getUri(),equalTo(uri)); assertThat(es.getPassword(), equalTo("pwd")); assertThat(es.getUsername(), equalTo("user")); } diff --git a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java index bb719a0a..12bd6886 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java @@ -46,7 +46,7 @@ public class LogstashIntegrationTest @Before public void setup() throws Exception { - memoryDao = new MemoryDao("localhost", 4567); + memoryDao = new MemoryDao(); PowerMockito.mockStatic(LogstashConfiguration.class); when(LogstashConfiguration.getInstance()).thenReturn(logstashConfiguration); when(logstashConfiguration.getIndexerInstance()).thenReturn(memoryDao); diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java index ea820263..62ffb59f 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -1,7 +1,12 @@ package jenkins.plugins.logstash.configuration; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +import java.net.URI; +import java.net.URISyntaxException; import org.junit.Before; import org.junit.Rule; @@ -18,21 +23,16 @@ public class ElasticSearchTest private ElasticSearch indexer2; @Before - public void setup() + public void setup() throws URISyntaxException { - indexer = new ElasticSearch(); - indexer.setHost("http://localhost"); - indexer.setPort(4567); + String uri = "http://localhost:4567/key"; + indexer = new ElasticSearch(uri); indexer.setPassword("password"); indexer.setUsername("user"); - indexer.setKey("key"); - indexer2 = new ElasticSearch(); - indexer2.setHost("http://localhost"); - indexer2.setPort(4567); + indexer2 = new ElasticSearch(uri); indexer2.setPassword("password"); indexer2.setUsername("user"); - indexer2.setKey("key"); } @Test @@ -48,6 +48,13 @@ public void passwordChangeIsNotEqual() assertThat(indexer.equals(indexer2), is(false)); } + @Test + public void uriChangeIsNotEqual() throws URISyntaxException + { + indexer.setUri("https://localhost:4567/key"); + assertThat(indexer.equals(indexer2), is(false)); + } + @Test public void usernameChangeIsNotEqual() { @@ -56,10 +63,44 @@ public void usernameChangeIsNotEqual() } @Test - public void keyChangeIsNotEqual() + public void missingPathThrowsException() throws URISyntaxException { - indexer.setKey("newKey"); - assertThat(indexer.equals(indexer2), is(false)); + try + { + indexer.setUri("http://localhost:8000/"); + fail("Expected an IllegalArgumentException"); + } + catch (IllegalArgumentException e) + { + assertThat("Please specify an elastic search key.",equalTo(e.getMessage())); + } } + @Test + public void missingPortThrowsException() throws URISyntaxException + { + try + { + indexer.setUri("http://localhost/logstash"); + fail("Expected an IllegalArgumentException"); + } + catch (IllegalArgumentException e) + { + assertThat("Please specify a port.",equalTo(e.getMessage())); + } + } + + @Test + public void missingSchemeThrowsException() throws URISyntaxException + { + try + { + indexer.setUri("localhost:8000/logstash"); + fail("Expected an IllegalArgumentException"); + } + catch (IllegalArgumentException e) + { + assertThat("unknown protocol: localhost",equalTo(e.getMessage())); + } + } } diff --git a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java b/src/test/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexerTest.java similarity index 86% rename from src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java rename to src/test/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexerTest.java index 9684048c..7d203add 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/LogstashIndexerTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/HostBasedLogstashIndexerTest.java @@ -8,7 +8,7 @@ import jenkins.plugins.logstash.persistence.MemoryDao; -public class LogstashIndexerTest +public class HostBasedLogstashIndexerTest { private LogstashIndexerForTest indexer; @@ -41,7 +41,7 @@ public void portChangeIsNotEqual() assertThat(indexer.equals(indexer2), is(false)); } - public static class LogstashIndexerForTest extends LogstashIndexer + public static class LogstashIndexerForTest extends HostBasedLogstashIndexer { public LogstashIndexerForTest(String host, int port) @@ -53,7 +53,7 @@ public LogstashIndexerForTest(String host, int port) @Override public MemoryDao createIndexerInstance() { - return new MemoryDao(getHost(), getPort()); + return new MemoryDao(); } } } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java index 7ab763d0..11de969e 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDaoTest.java @@ -66,9 +66,15 @@ public void buildPayloadSuccessTwoLines() throws Exception { } private AbstractLogstashIndexerDao getInstance() { - return new AbstractLogstashIndexerDao("localhost", -1) { + return new AbstractLogstashIndexerDao() { @Override public void push(String data) throws IOException {} + + @Override + public String getDescription() + { + return "test"; + } }; } } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java index 83df99b6..3f249c74 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java @@ -5,6 +5,7 @@ import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.utils.URIBuilder; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.HttpClientBuilder; @@ -19,9 +20,11 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) @@ -33,14 +36,15 @@ public class ElasticSearchDaoTest { @Mock CloseableHttpResponse mockResponse; @Mock HttpEntity mockEntity; - ElasticSearchDao createDao(String host, int port, String key, String username, String password) { - return new ElasticSearchDao(mockClientBuilder, host, port, key, username, password); + ElasticSearchDao createDao(String url, String username, String password) throws URISyntaxException { + URI uri = new URI(url); + return new ElasticSearchDao(mockClientBuilder, uri, username, password); } @Before public void before() throws Exception { int port = (int) (Math.random() * 1000); - dao = createDao("http://localhost", port, "/jenkins/logstash", "username", "password"); + dao = createDao("http://localhost:8200/logstash", "username", "password"); when(mockClientBuilder.build()).thenReturn(mockHttpClient); when(mockHttpClient.execute(any(HttpPost.class))).thenReturn(mockResponse); @@ -53,65 +57,35 @@ public void after() throws Exception { verifyNoMoreInteractions(mockHttpClient); } - @Test(expected = IllegalArgumentException.class) - public void constructorFailNullHost() throws Exception { - try { - createDao(null, 8200, "logstash", "username", "password"); - } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "host name is required", e.getMessage()); - throw e; - } - } - - @Test(expected = IllegalArgumentException.class) - public void constructorFailEmptyHost() throws Exception { - try { - createDao(" ", 8200, "logstash", "username", "password"); - } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "host name is required", e.getMessage()); - throw e; - } - } - - @Test(expected = IllegalArgumentException.class) + @Test public void constructorFailMissingScheme() throws Exception { try { - createDao("localhost", 8200, "logstash", "username", "password"); - } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "host field must specify scheme, such as 'http://'", e.getMessage()); - throw e; - } - } - - @Test(expected = IllegalArgumentException.class) - public void constructorFailNullKey() throws Exception { - try { - createDao("http://localhost", 8200, null, "username", "password"); + createDao("localhost:8200/logstash", "username", "password"); + fail("Expected an IllegalArgumentException."); } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "elastic index name is required", e.getMessage()); - throw e; + assertEquals("Wrong error message was thrown", "unknown protocol: localhost", e.getMessage()); } } - @Test(expected = IllegalArgumentException.class) + @Test public void constructorFailEmptyKey() throws Exception { try { - createDao("http://localhost", 8200, " ", "username", "password"); + createDao("http://localhost:8200", "username", "password"); + fail("Expected an IllegalArgumentException."); } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "elastic index name is required", e.getMessage()); - throw e; + assertEquals("Wrong error message was thrown", "Please specify an elastic search key.", e.getMessage()); } } @Test public void constructorSuccess1() throws Exception { // Unit under test - dao = createDao("https://localhost", 8200, "logstash", "username", "password"); + dao = createDao("https://localhost:8200/logstash", "username", "password"); // Verify results - assertEquals("Wrong host name", "https://localhost", dao.getHost()); + assertEquals("Wrong host name", "localhost", dao.getHost()); assertEquals("Wrong port", 8200, dao.getPort()); - assertEquals("Wrong key", "logstash", dao.getKey()); + assertEquals("Wrong key", "/logstash", dao.getKey()); assertEquals("Wrong name", "username", dao.getUsername()); assertEquals("Wrong password", "password", dao.getPassword()); assertEquals("Wrong auth", "dXNlcm5hbWU6cGFzc3dvcmQ=", dao.getAuth()); @@ -121,12 +95,13 @@ public void constructorSuccess1() throws Exception { @Test public void constructorSuccess2() throws Exception { // Unit under test - dao = createDao("http://localhost", 8200, "jenkins/logstash", "", "password"); + dao = createDao("http://localhost:8200/jenkins/logstash", "", "password"); // Verify results - assertEquals("Wrong host name", "http://localhost", dao.getHost()); + assertEquals("Wrong host name", "localhost", dao.getHost()); assertEquals("Wrong port", 8200, dao.getPort()); - assertEquals("Wrong key", "jenkins/logstash", dao.getKey()); + assertEquals("Wrong scheme", "http", dao.getScheme()); + assertEquals("Wrong key", "/jenkins/logstash", dao.getKey()); assertEquals("Wrong name", "", dao.getUsername()); assertEquals("Wrong password", "password", dao.getPassword()); assertEquals("Wrong auth", null, dao.getAuth()); @@ -136,11 +111,12 @@ public void constructorSuccess2() throws Exception { @Test public void constructorSuccess3() throws Exception { // Unit under test - dao = createDao("http://localhost", 8200, "/jenkins//logstash/", "userlongername", null); + dao = createDao("http://localhost:8200/jenkins//logstash/", "userlongername", null); // Verify results - assertEquals("Wrong host name", "http://localhost", dao.getHost()); + assertEquals("Wrong host name", "localhost", dao.getHost()); assertEquals("Wrong port", 8200, dao.getPort()); + assertEquals("Wrong scheme", "http", dao.getScheme()); assertEquals("Wrong key", "/jenkins//logstash/", dao.getKey()); assertEquals("Wrong name", "userlongername", dao.getUsername()); assertEquals("Wrong password", null, dao.getPassword()); @@ -151,7 +127,7 @@ public void constructorSuccess3() throws Exception { @Test public void getPostSuccessNoAuth() throws Exception { String json = "{ 'foo': 'bar' }"; - dao = createDao("http://localhost", 8200, "/jenkins/logstash", "", ""); + dao = createDao("http://localhost:8200/jenkins/logstash", "", ""); // Unit under test HttpPost post = dao.getHttpPost(json); @@ -170,7 +146,7 @@ public void getPostSuccessNoAuth() throws Exception { @Test public void getPostSuccessAuth() throws Exception { String json = "{ 'foo': 'bar' }"; - dao = createDao("https://localhost", 8200, "/jenkins/logstash", "username", "password"); + dao = createDao("https://localhost:8200/jenkins/logstash", "username", "password"); // Unit under test HttpPost post = dao.getHttpPost(json); @@ -192,7 +168,7 @@ public void getPostSuccessAuth() throws Exception { @Test public void pushSuccess() throws Exception { String json = "{ 'foo': 'bar' }"; - dao = createDao("http://localhost", 8200, "/jenkins/logstash", "", ""); + dao = createDao("http://localhost:8200/jenkins/logstash", "", ""); when(mockStatusLine.getStatusCode()).thenReturn(201); @@ -209,7 +185,7 @@ public void pushSuccess() throws Exception { @Test(expected = IOException.class) public void pushFailStatusCode() throws Exception { String json = "{ 'foo': 'bar' }"; - dao = createDao("http://localhost", 8200, "/jenkins/logstash", "username", "password"); + dao = createDao("http://localhost:8200/jenkins/logstash", "username", "password"); when(mockStatusLine.getStatusCode()).thenReturn(500); when(mockResponse.getEntity()).thenReturn(new StringEntity("Something bad happened.", ContentType.TEXT_PLAIN)); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java b/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java index 502c6b7f..0738eb7d 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/MemoryDao.java @@ -11,9 +11,9 @@ public class MemoryDao extends AbstractLogstashIndexerDao { List output = new ArrayList<>(); - public MemoryDao(String host, int port) + public MemoryDao() { - super(host, port); + super(); } @Override @@ -27,4 +27,10 @@ public List getOutput() { return output; } + + @Override + public String getDescription() + { + return "test"; + } } \ No newline at end of file diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java index db6d4223..1ac4ad2a 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java @@ -2,6 +2,8 @@ import com.cloudbees.syslog.MessageFormat; import com.cloudbees.syslog.sender.UdpSyslogMessageSender; + +import java.io.File; import java.io.FileNotFoundException; import java.io.PrintWriter; import java.nio.file.Files; @@ -41,6 +43,8 @@ public void syslogSendRFC3164UDP() throws Exception { // Clean up the the logstash log file try { + File file = new File(logfile); + file.getParentFile().mkdirs(); writer = new PrintWriter(logfile); } catch (FileNotFoundException e) { diff --git a/src/test/resources/elasticSearch.xml b/src/test/resources/elasticSearch.xml index 3f3d7c7e..494e0bf0 100644 --- a/src/test/resources/elasticSearch.xml +++ b/src/test/resources/elasticSearch.xml @@ -1,9 +1,7 @@ - http://localhost - 9300 - logstash + http://localhost:9300/logstash {AQAAABAAAAAQdB13xsx5UlCScGiBcVUOL0GqWYwAU5syhW9iBb6tG+4=} From f021a434072add61c1b73c4e430a790d4507893f Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 4 Jan 2018 01:21:39 +0100 Subject: [PATCH 13/17] add some todos --- .../java/jenkins/plugins/logstash/LogstashWriter.java | 3 +++ .../logstash/persistence/AbstractLogstashIndexerDao.java | 2 ++ .../plugins/logstash/persistence/RabbitMqDao.java | 8 +++++++- .../jenkins/plugins/logstash/persistence/RedisDao.java | 6 +++++- .../jenkins/plugins/logstash/persistence/SyslogDao.java | 9 +++++++++ 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/LogstashWriter.java b/src/main/java/jenkins/plugins/logstash/LogstashWriter.java index a9ead06a..c60652c2 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashWriter.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashWriter.java @@ -64,6 +64,9 @@ public class LogstashWriter { private boolean connectionBroken; private Charset charset; + /* + * TODO: the charset must not be transfered to the dao. The dao is shared between different build. + */ public LogstashWriter(Run run, OutputStream error, TaskListener listener, Charset charset) { this.errorStream = error != null ? error : System.err; this.build = run; diff --git a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java index 9b6f48a2..f08d690b 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/AbstractLogstashIndexerDao.java @@ -35,6 +35,8 @@ /** * Abstract data access object for Logstash indexers. * + * TODO: a charset is only required for RabbitMq currently (ES as well but there it is currently configured via the ContentType), + * so better move this to the corresponding classes. * @author Rusty Gerard * @since 1.0.0 */ diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java index 61d9fae5..8eca8ff9 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java @@ -35,6 +35,8 @@ /** * RabbitMQ Data Access Object. * + * TODO: make the charset configurable via the UI with UTF-8 being the default + * * @author Rusty Gerard * @since 1.0.0 */ @@ -50,7 +52,11 @@ public RabbitMqDao(String host, int port, String key, String username, String pa this(null, host, port, key, username, password); } - // Factored for unit testing + /* + * TODO: this constructor is only for testing so one can inject a mocked ConnectionFactory. + * With Powermock we can intercept the creation of the ConnectionFactory and replace with a mock + * making this constructor obsolete + */ RabbitMqDao(ConnectionFactory factory, String host, int port, String queue, String username, String password) { super(host, port); diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java index e0fbb7d7..b36a73a0 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RedisDao.java @@ -51,7 +51,11 @@ public RedisDao(String host, int port, String key, String password) { this(null, host, port, key, password); } - // Factored for unit testing + /* + * TODO: this constructor is only for testing so one can inject a mocked JedisPool. + * With Powermock we can intercept the creation of the JedisPool and replace with a mock + * making this constructor obsolete + */ RedisDao(JedisPool factory, String host, int port, String key, String password) { super(host, port); diff --git a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java index 70c5b105..86f96d41 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/SyslogDao.java @@ -7,6 +7,9 @@ import com.cloudbees.syslog.Severity; import com.cloudbees.syslog.sender.UdpSyslogMessageSender; +/* + * TODO: add support for TcpSyslogMessageSender + */ public class SyslogDao extends HostBasedLogstashIndexerDao { private MessageFormat messageFormat = MessageFormat.RFC_3164; @@ -16,6 +19,12 @@ public SyslogDao(String host, int port) { this(null, host, port); } + /* + * TODO: this constructor is only for testing so one can inject a mocked UdpSyslogMessageSender. + * With Powermock we can intercept the creation of the UdpSyslogMessageSender and replace with a mock + * making this constructor obsolete + */ + public SyslogDao(UdpSyslogMessageSender udpSyslogMessageSender, String host, int port) { super(host, port); messageSender = udpSyslogMessageSender == null ? new UdpSyslogMessageSender() : udpSyslogMessageSender; From fdc79cc4524d3e08cd5537f8b0622184cab7985e Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Fri, 5 Jan 2018 00:04:20 +0100 Subject: [PATCH 14/17] use URL instead of URI stapler is not able to autconvert String to URI add more TODOs --- .../logstash/LogstashConfiguration.java | 6 +- .../jenkins/plugins/logstash/PluginImpl.java | 13 ++- .../logstash/configuration/ElasticSearch.java | 95 +++++++++++-------- .../configuration/HostConfiguration.java | 66 ------------- .../configuration/LogstashIndexer.java | 2 +- .../persistence/ElasticSearchDao.java | 24 ++--- .../HostBasedLogstashIndexerDao.java | 2 - .../logstash/persistence/RabbitMqDao.java | 15 ++- .../configuration/ElasticSearch/config.jelly | 2 +- .../LogstashConfigurationMigrationTest.java | 10 +- .../logstash/LogstashConfigurationTest.java | 1 - .../configuration/ElasticSearchTest.java | 57 ++--------- .../persistence/ElasticSearchDaoTest.java | 17 +--- src/test/resources/elasticSearch.xml | 2 +- 14 files changed, 111 insertions(+), 201 deletions(-) delete mode 100644 src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 0807a6cf..78124a54 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -1,5 +1,6 @@ package jenkins.plugins.logstash; +import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.util.List; @@ -108,12 +109,13 @@ public void migrateData() uri = (new URIBuilder(descriptor.getHost())) .setPort(descriptor.getPort()) .setPath("/" + descriptor.getKey()).build(); - ElasticSearch es = new ElasticSearch(uri.toString()); + ElasticSearch es = new ElasticSearch(); + es.setUrl(uri.toURL()); es.setUsername(descriptor.getUsername()); es.setPassword(descriptor.getPassword()); logstashIndexer = es; } - catch (URISyntaxException e) + catch (URISyntaxException | MalformedURLException e) { LOGGER.log(Level.INFO, "Migrating logstash configuration for Elastic Search failed: " + e.toString()); } diff --git a/src/main/java/jenkins/plugins/logstash/PluginImpl.java b/src/main/java/jenkins/plugins/logstash/PluginImpl.java index d11216af..e7c15a6e 100644 --- a/src/main/java/jenkins/plugins/logstash/PluginImpl.java +++ b/src/main/java/jenkins/plugins/logstash/PluginImpl.java @@ -1,18 +1,18 @@ /* * The MIT License - * + * * Copyright 2013 Hewlett-Packard Development Company, L.P. - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal * in the Software without restriction, including without limitation the rights * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell * copies of the Software, and to permit persons to whom the Software is * furnished to do so, subject to the following conditions: - * + * * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -27,9 +27,14 @@ import java.util.logging.Logger; +/* + * TODO: do we really need this class? + * All it does is printing a message at startup of Jenkins. + */ public class PluginImpl extends Plugin { private final static Logger LOG = Logger.getLogger(PluginImpl.class.getName()); + @Override public void start() throws Exception { LOG.info("Logstash: a logstash agent to send jenkins logs to a logstash indexer."); } diff --git a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java index 502cf1a8..147d960c 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java @@ -3,6 +3,7 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.DataBoundConstructor; @@ -19,25 +20,41 @@ public class ElasticSearch extends LogstashIndexer { private String username; private Secret password; - private URI uri; + private URL url; @DataBoundConstructor - public ElasticSearch(String uri) throws URISyntaxException + public ElasticSearch() { - this.uri = new URI(uri); - validateUri(this.uri); } - public URI getUri() + public URL getUrl() { - return uri; + return url; } - public void setUri(String value) throws URISyntaxException + + private URI getUri() + { + if (url != null) + { + URI uri; + try + { + uri = url.toURI(); + return uri; + } + catch (URISyntaxException e) + { + return null; + } + } + return null; + } + + @DataBoundSetter + public void setUrl(URL uri) { - URI uri = new URI(value); - validateUri(uri); - this.uri = uri; + this.url = uri; } public String getUsername() @@ -76,12 +93,14 @@ public boolean equals(Object obj) { return false; } - if (uri == null) + if (url == null) { - if (other.uri != null) + if (other.url != null) return false; } - else if (!uri.equals(other.uri)) + // String comparison is not optimal but comparing the urls directly is + // criticized by findbugs as being a blocking operation + else if (!url.toString().equals(other.url.toString())) { return false; } @@ -102,7 +121,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); - result = prime * result + ((uri == null) ? 0 : uri.hashCode()); + result = prime * result + ((url == null) ? 0 : url.toString().hashCode()); result = prime * result + ((username == null) ? 0 : username.hashCode()); result = prime * result + Secret.toString(password).hashCode(); return result; @@ -111,7 +130,7 @@ public int hashCode() @Override public ElasticSearchDao createIndexerInstance() { - return new ElasticSearchDao(uri, username, Secret.toString(password)); + return new ElasticSearchDao(getUri(), username, Secret.toString(password)); } @Extension @@ -129,7 +148,7 @@ public int getDefaultPort() return 9300; } - public FormValidation doCheckUri(@QueryParameter("value") String value) + public FormValidation doCheckUrl(@QueryParameter("value") String value) { if (StringUtils.isBlank(value)) { @@ -137,10 +156,20 @@ public FormValidation doCheckUri(@QueryParameter("value") String value) } try { - URI uri = new URI(value); - validateUri(uri); + URL url = new URL(value); + + if (url.getUserInfo() != null) + { + return FormValidation.error("Please specify user and password not as part of the url."); + } + + if(StringUtils.isBlank(url.getPath()) || url.getPath().trim().matches("^\\/+$")) { + return FormValidation.warning("Elastic Search requires a key to be able to index the logs."); + } + + url.toURI(); } - catch (URISyntaxException | IllegalArgumentException e) + catch (MalformedURLException | URISyntaxException e) { return FormValidation.error(e.getMessage()); } @@ -148,29 +177,13 @@ public FormValidation doCheckUri(@QueryParameter("value") String value) } } - /** - * Validates that the given uri has a scheme, a port and a path which is not empty or just "/" - * - * @param uri - * @throws IllegalArgumentException when one of scheme, port or path - */ - public static void validateUri(URI uri) throws IllegalArgumentException + public static void main(String[] args) throws MalformedURLException, URISyntaxException { - try - { - uri.toURL(); - } - catch (MalformedURLException e) - { - throw new IllegalArgumentException(e.getMessage()); - } + URL url = new URL("localhost/logstash"); + System.out.println("Path: " + url.getPath()); + System.out.println(url.toURI().getUserInfo()); + System.out.println(url.toURI().getAuthority()); + System.out.println(url.toURI().getHost()); - if(uri.getPort() == -1) { - throw new IllegalArgumentException("Please specify a port."); - } - - if(StringUtils.isBlank(uri.getPath()) || uri.getPath().trim().matches("^\\/+$")) { - throw new IllegalArgumentException("Please specify an elastic search key."); - } } } diff --git a/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java b/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java deleted file mode 100644 index 907030f3..00000000 --- a/src/main/java/jenkins/plugins/logstash/configuration/HostConfiguration.java +++ /dev/null @@ -1,66 +0,0 @@ -package jenkins.plugins.logstash.configuration; - -import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; - -public class HostConfiguration -{ - - protected String host; - protected int port; - - @DataBoundConstructor - public HostConfiguration() - { - } - - public String getHost() - { - return host; - } - - @DataBoundSetter - public void setHost(String host) - { - this.host = host; - } - - public int getPort() - { - return port; - } - - @DataBoundSetter - public void setPort(int port) - { - this.port = port; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((host == null) ? 0 : host.hashCode()); - result = prime * result + port; - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - HostConfiguration other = (HostConfiguration) obj; - if (host == null) { - if (other.host != null) - return false; - } else if (!host.equals(other.host)) - return false; - if (port != other.port) - return false; - return true; - } -} diff --git a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java index 94acc951..6f3955b5 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/LogstashIndexer.java @@ -39,7 +39,7 @@ public abstract class LogstashIndexer * @return {@link AbstractLogstashIndexerDao} instance */ @Nonnull - public synchronized final T getInstance() + public synchronized T getInstance() { if (instance == null) { diff --git a/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java b/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java index 5dba81bd..e5167f21 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/ElasticSearchDao.java @@ -83,7 +83,15 @@ public ElasticSearchDao(URI uri, String username, String password) { this.username = username; this.password = password; - ElasticSearch.validateUri(uri); + + try + { + uri.toURL(); + } + catch (MalformedURLException e) + { + throw new IllegalArgumentException(e); + } if (StringUtils.isNotBlank(username)) { auth = Base64.encodeBase64String((username + ":" + StringUtils.defaultString(password)).getBytes(StandardCharsets.UTF_8)); @@ -198,20 +206,6 @@ private String getErrorMessage(CloseableHttpResponse response) { } } - public static void main(String[] args) throws URISyntaxException, MalformedURLException - { - URI uri = (new URIBuilder("localhost:8000")).setPath("/logstash").build(); - - //System.out.println(uri.toURL()); - System.out.println("host:" + uri.getHost()); - System.out.println("port:" + uri.getPort()); - System.out.println("path:" + uri.getPath()); - System.out.println("scheme:" + uri.getScheme()); - System.out.println("fragment:" + uri.getFragment()); - System.out.println("rawpath:" + uri.getRawPath()); - - } - @Override public String getDescription() { diff --git a/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java index 6d11d0dc..05ba6751 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java @@ -1,8 +1,6 @@ /* * The MIT License * - * Copyright 2014 Rusty Gerard - * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal * in the Software without restriction, including without limitation the rights diff --git a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java index 8eca8ff9..62f8ec86 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/RabbitMqDao.java @@ -36,6 +36,8 @@ * RabbitMQ Data Access Object. * * TODO: make the charset configurable via the UI with UTF-8 being the default + * TODO: support TLS + * TODO: support vhost * * @author Rusty Gerard * @since 1.0.0 @@ -97,6 +99,15 @@ public String getPassword() } + /* + * TODO: do we really need to open a connection each time? + * channels are not thread-safe so we need a new channel each time but the connection + * could be shared. Another idea would be to use one dao per build, reuse the channel and + * synchronize the push on the channel (for pipeline builds where we can have multiple + * threads writing, are freestyle projects guaranteed to be single threaded?). + * (non-Javadoc) + * @see jenkins.plugins.logstash.persistence.LogstashIndexerDao#push(java.lang.String) + */ @Override public void push(String data) throws IOException { Connection connection = null; @@ -104,8 +115,8 @@ public void push(String data) throws IOException { try { connection = pool.newConnection(); channel = connection.createChannel(); - // Ensure the queue exists + try { channel.queueDeclarePassive(queue); } catch (IOException e) { @@ -124,6 +135,7 @@ public void push(String data) throws IOException { } } + // TODO: connection.isOpen() should be avoided (see the rabbitmq doc) private void finalizeConnection(Connection connection) { if (connection != null && connection.isOpen()) { try { @@ -135,6 +147,7 @@ private void finalizeConnection(Connection connection) { } } + // TODO: connection.isOpen() should be avoided (see the rabbitmq doc) private void finalizeChannel(Channel channel) { if (channel != null && channel.isOpen()) { try { diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly index a01fab63..3b4683f2 100644 --- a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly @@ -1,6 +1,6 @@ - + diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java index 9cd5cbd3..6e1ab2ea 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java @@ -7,10 +7,10 @@ import static org.powermock.api.mockito.PowerMockito.when; import java.io.File; -import java.net.URI; +import java.net.MalformedURLException; import java.net.URISyntaxException; +import java.net.URL; -import org.apache.http.client.utils.URIBuilder; import org.hamcrest.core.IsInstanceOf; import org.junit.Before; import org.junit.Rule; @@ -104,7 +104,7 @@ public void syslogMigrationRFC5424() } @Test - public void elasticSearchMigration() throws URISyntaxException + public void elasticSearchMigration() throws URISyntaxException, MalformedURLException { when(descriptor.getType()).thenReturn(IndexerType.ELASTICSEARCH); when(descriptor.getHost()).thenReturn("http://localhost"); @@ -112,8 +112,8 @@ public void elasticSearchMigration() throws URISyntaxException LogstashIndexer indexer = configuration.getLogstashIndexer(); assertThat(indexer, IsInstanceOf.instanceOf(ElasticSearch.class)); ElasticSearch es = (ElasticSearch) indexer; - URI uri = (new URIBuilder("http://localhost")).setPort(4567).setPath("/logstash").build(); - assertThat(es.getUri(),equalTo(uri)); + URL url = new URL("http://localhost:4567/logstash"); + assertThat(es.getUrl(),equalTo(url)); assertThat(es.getPassword(), equalTo("pwd")); assertThat(es.getUsername(), equalTo("user")); } diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java index 70dd3912..a992d388 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationTest.java @@ -1,7 +1,6 @@ package jenkins.plugins.logstash; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import java.io.File; diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java index 62ffb59f..d566d3e0 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -5,8 +5,10 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; import org.junit.Before; import org.junit.Rule; @@ -23,14 +25,16 @@ public class ElasticSearchTest private ElasticSearch indexer2; @Before - public void setup() throws URISyntaxException + public void setup() throws MalformedURLException { - String uri = "http://localhost:4567/key"; - indexer = new ElasticSearch(uri); + URL url = new URL("http://localhost:4567/key"); + indexer = new ElasticSearch(); + indexer.setUrl(url); indexer.setPassword("password"); indexer.setUsername("user"); - indexer2 = new ElasticSearch(uri); + indexer2 = new ElasticSearch(); + indexer2.setUrl(url); indexer2.setPassword("password"); indexer2.setUsername("user"); } @@ -49,9 +53,9 @@ public void passwordChangeIsNotEqual() } @Test - public void uriChangeIsNotEqual() throws URISyntaxException + public void urlChangeIsNotEqual() throws MalformedURLException { - indexer.setUri("https://localhost:4567/key"); + indexer.setUrl(new URL("https://localhost:4567/key")); assertThat(indexer.equals(indexer2), is(false)); } @@ -62,45 +66,4 @@ public void usernameChangeIsNotEqual() assertThat(indexer.equals(indexer2), is(false)); } - @Test - public void missingPathThrowsException() throws URISyntaxException - { - try - { - indexer.setUri("http://localhost:8000/"); - fail("Expected an IllegalArgumentException"); - } - catch (IllegalArgumentException e) - { - assertThat("Please specify an elastic search key.",equalTo(e.getMessage())); - } - } - - @Test - public void missingPortThrowsException() throws URISyntaxException - { - try - { - indexer.setUri("http://localhost/logstash"); - fail("Expected an IllegalArgumentException"); - } - catch (IllegalArgumentException e) - { - assertThat("Please specify a port.",equalTo(e.getMessage())); - } - } - - @Test - public void missingSchemeThrowsException() throws URISyntaxException - { - try - { - indexer.setUri("localhost:8000/logstash"); - fail("Expected an IllegalArgumentException"); - } - catch (IllegalArgumentException e) - { - assertThat("unknown protocol: localhost",equalTo(e.getMessage())); - } - } } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java index 3f249c74..84b7c19f 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java @@ -5,7 +5,6 @@ import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.utils.URIBuilder; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.HttpClientBuilder; @@ -58,22 +57,12 @@ public void after() throws Exception { } @Test - public void constructorFailMissingScheme() throws Exception { + public void constructorFailInvalidUrl() throws Exception { try { - createDao("localhost:8200/logstash", "username", "password"); + createDao("localhost:8000/logstash", "username", "password"); fail("Expected an IllegalArgumentException."); } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "unknown protocol: localhost", e.getMessage()); - } - } - - @Test - public void constructorFailEmptyKey() throws Exception { - try { - createDao("http://localhost:8200", "username", "password"); - fail("Expected an IllegalArgumentException."); - } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "Please specify an elastic search key.", e.getMessage()); + assertEquals("Wrong error message was thrown", "java.net.MalformedURLException: unknown protocol: localhost", e.getMessage()); } } diff --git a/src/test/resources/elasticSearch.xml b/src/test/resources/elasticSearch.xml index 494e0bf0..203a3268 100644 --- a/src/test/resources/elasticSearch.xml +++ b/src/test/resources/elasticSearch.xml @@ -1,7 +1,7 @@ - http://localhost:9300/logstash + http://localhost:9300/logstash {AQAAABAAAAAQdB13xsx5UlCScGiBcVUOL0GqWYwAU5syhW9iBb6tG+4=} From dac27f1ee6dcb5926fa1a5a66418ee2bf1f245e8 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Wed, 10 Jan 2018 22:32:49 +0100 Subject: [PATCH 15/17] use URI as persistence for elastic search url URI can be easier compared some minor changes in tests --- .../logstash/LogstashConfiguration.java | 14 ++--- .../logstash/configuration/ElasticSearch.java | 54 ++++++------------- .../HostBasedLogstashIndexerDao.java | 3 +- .../configuration/ElasticSearch/config.jelly | 2 +- .../LogstashConfigurationMigrationTest.java | 5 +- .../configuration/ElasticSearchTest.java | 10 ++-- .../persistence/ElasticSearchDaoTest.java | 3 +- .../logstash/persistence/SyslogDaoTestIT.java | 2 - 8 files changed, 35 insertions(+), 58 deletions(-) diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java index 78124a54..bbd68a7c 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConfiguration.java @@ -40,10 +40,7 @@ public class LogstashConfiguration extends GlobalConfiguration public LogstashConfiguration() { load(); - if (logstashIndexer != null) - { - activeIndexer = logstashIndexer; - } + activeIndexer = logstashIndexer; } /** @@ -110,12 +107,12 @@ public void migrateData() .setPort(descriptor.getPort()) .setPath("/" + descriptor.getKey()).build(); ElasticSearch es = new ElasticSearch(); - es.setUrl(uri.toURL()); + es.setUri(uri); es.setUsername(descriptor.getUsername()); es.setPassword(descriptor.getPassword()); logstashIndexer = es; } - catch (URISyntaxException | MalformedURLException e) + catch (URISyntaxException e) { LOGGER.log(Level.INFO, "Migrating logstash configuration for Elastic Search failed: " + e.toString()); } @@ -164,7 +161,10 @@ public void migrateData() @Override public boolean configure(StaplerRequest staplerRequest, JSONObject json) throws FormException { - // when we bind the stapler request we get a new instance of logstashIndexer + // when we bind the stapler request we get a new instance of logstashIndexer. + // logstashIndexer is holder for the dao instance. + // To avoid that we get a new dao instance in case there was no change in configuration + // we compare it to the currently active configuration. staplerRequest.bindJSON(this, json); if (!Objects.equals(logstashIndexer, activeIndexer)) { diff --git a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java index 147d960c..f5f1a7db 100644 --- a/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java +++ b/src/main/java/jenkins/plugins/logstash/configuration/ElasticSearch.java @@ -20,41 +20,31 @@ public class ElasticSearch extends LogstashIndexer { private String username; private Secret password; - private URL url; + private URI uri; @DataBoundConstructor public ElasticSearch() { } - public URL getUrl() + public URI getUri() { - return url; + return uri; } - private URI getUri() + /* + * We use URL for the setter as stapler can autoconvert a string to a URL but not to a URI + */ + @DataBoundSetter + public void setUri(URL url) throws URISyntaxException { - if (url != null) - { - URI uri; - try - { - uri = url.toURI(); - return uri; - } - catch (URISyntaxException e) - { - return null; - } - } - return null; + this.uri = url.toURI(); } - @DataBoundSetter - public void setUrl(URL uri) + public void setUri(URI uri) throws URISyntaxException { - this.url = uri; + this.uri = uri; } public String getUsername() @@ -93,14 +83,12 @@ public boolean equals(Object obj) { return false; } - if (url == null) + if (uri == null) { - if (other.url != null) + if (other.uri != null) return false; } - // String comparison is not optimal but comparing the urls directly is - // criticized by findbugs as being a blocking operation - else if (!url.toString().equals(other.url.toString())) + else if (!uri.equals(other.uri)) { return false; } @@ -121,7 +109,7 @@ public int hashCode() { final int prime = 31; int result = super.hashCode(); - result = prime * result + ((url == null) ? 0 : url.toString().hashCode()); + result = prime * result + ((uri == null) ? 0 : uri.hashCode()); result = prime * result + ((username == null) ? 0 : username.hashCode()); result = prime * result + Secret.toString(password).hashCode(); return result; @@ -145,7 +133,7 @@ public String getDisplayName() @Override public int getDefaultPort() { - return 9300; + return 0; } public FormValidation doCheckUrl(@QueryParameter("value") String value) @@ -176,14 +164,4 @@ public FormValidation doCheckUrl(@QueryParameter("value") String value) return FormValidation.ok(); } } - - public static void main(String[] args) throws MalformedURLException, URISyntaxException - { - URL url = new URL("localhost/logstash"); - System.out.println("Path: " + url.getPath()); - System.out.println(url.toURI().getUserInfo()); - System.out.println(url.toURI().getAuthority()); - System.out.println(url.toURI().getHost()); - - } } diff --git a/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java index 05ba6751..92feb616 100644 --- a/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java +++ b/src/main/java/jenkins/plugins/logstash/persistence/HostBasedLogstashIndexerDao.java @@ -33,8 +33,7 @@ /** * Abstract data access object for Logstash indexers. * - * @author Rusty Gerard - * @since 1.0.0 + * @since 2.0.0 */ public abstract class HostBasedLogstashIndexerDao extends AbstractLogstashIndexerDao { private final String host; diff --git a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly index 3b4683f2..a01fab63 100644 --- a/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly +++ b/src/main/resources/jenkins/plugins/logstash/configuration/ElasticSearch/config.jelly @@ -1,6 +1,6 @@ - + diff --git a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java index 6e1ab2ea..a89ce386 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashConfigurationMigrationTest.java @@ -8,6 +8,7 @@ import java.io.File; import java.net.MalformedURLException; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -112,8 +113,8 @@ public void elasticSearchMigration() throws URISyntaxException, MalformedURLExce LogstashIndexer indexer = configuration.getLogstashIndexer(); assertThat(indexer, IsInstanceOf.instanceOf(ElasticSearch.class)); ElasticSearch es = (ElasticSearch) indexer; - URL url = new URL("http://localhost:4567/logstash"); - assertThat(es.getUrl(),equalTo(url)); + URI uri = new URI("http://localhost:4567/logstash"); + assertThat(es.getUri(),equalTo(uri)); assertThat(es.getPassword(), equalTo("pwd")); assertThat(es.getUsername(), equalTo("user")); } diff --git a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java index d566d3e0..171ed06e 100644 --- a/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java +++ b/src/test/java/jenkins/plugins/logstash/configuration/ElasticSearchTest.java @@ -25,16 +25,16 @@ public class ElasticSearchTest private ElasticSearch indexer2; @Before - public void setup() throws MalformedURLException + public void setup() throws MalformedURLException, URISyntaxException { URL url = new URL("http://localhost:4567/key"); indexer = new ElasticSearch(); - indexer.setUrl(url); + indexer.setUri(url); indexer.setPassword("password"); indexer.setUsername("user"); indexer2 = new ElasticSearch(); - indexer2.setUrl(url); + indexer2.setUri(url); indexer2.setPassword("password"); indexer2.setUsername("user"); } @@ -53,9 +53,9 @@ public void passwordChangeIsNotEqual() } @Test - public void urlChangeIsNotEqual() throws MalformedURLException + public void urlChangeIsNotEqual() throws MalformedURLException, URISyntaxException { - indexer.setUrl(new URL("https://localhost:4567/key")); + indexer.setUri(new URL("https://localhost:4567/key")); assertThat(indexer.equals(indexer2), is(false)); } diff --git a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java index 84b7c19f..d2af5787 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java @@ -58,8 +58,9 @@ public void after() throws Exception { @Test public void constructorFailInvalidUrl() throws Exception { + URI uri = new URI("localhost:8000/logstash"); try { - createDao("localhost:8000/logstash", "username", "password"); + new ElasticSearchDao(mockClientBuilder, uri, "username", "password"); fail("Expected an IllegalArgumentException."); } catch (IllegalArgumentException e) { assertEquals("Wrong error message was thrown", "java.net.MalformedURLException: unknown protocol: localhost", e.getMessage()); diff --git a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java index 1ac4ad2a..37d1684f 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/SyslogDaoTestIT.java @@ -43,8 +43,6 @@ public void syslogSendRFC3164UDP() throws Exception { // Clean up the the logstash log file try { - File file = new File(logfile); - file.getParentFile().mkdirs(); writer = new PrintWriter(logfile); } catch (FileNotFoundException e) { From f2c4a2474d1341ea5d0b75e407d5a6cc4a0ee3d6 Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 11 Jan 2018 00:04:05 +0100 Subject: [PATCH 16/17] fix test --- src/test/resources/elasticSearch.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/elasticSearch.xml b/src/test/resources/elasticSearch.xml index 203a3268..494e0bf0 100644 --- a/src/test/resources/elasticSearch.xml +++ b/src/test/resources/elasticSearch.xml @@ -1,7 +1,7 @@ - http://localhost:9300/logstash + http://localhost:9300/logstash {AQAAABAAAAAQdB13xsx5UlCScGiBcVUOL0GqWYwAU5syhW9iBb6tG+4=} From add814647c65710358602411862a8ecb36bd875e Mon Sep 17 00:00:00 2001 From: Markus Winter Date: Thu, 11 Jan 2018 17:09:59 +0100 Subject: [PATCH 17/17] use ExpectedException rule --- .../logstash/persistence/ElasticSearchDaoTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java index d2af5787..e18e1e88 100644 --- a/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java +++ b/src/test/java/jenkins/plugins/logstash/persistence/ElasticSearchDaoTest.java @@ -11,7 +11,9 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.*; import org.mockito.junit.MockitoJUnitRunner; @@ -35,6 +37,9 @@ public class ElasticSearchDaoTest { @Mock CloseableHttpResponse mockResponse; @Mock HttpEntity mockEntity; + @Rule + public ExpectedException thrown = ExpectedException.none(); + ElasticSearchDao createDao(String url, String username, String password) throws URISyntaxException { URI uri = new URI(url); return new ElasticSearchDao(mockClientBuilder, uri, username, password); @@ -59,12 +64,9 @@ public void after() throws Exception { @Test public void constructorFailInvalidUrl() throws Exception { URI uri = new URI("localhost:8000/logstash"); - try { - new ElasticSearchDao(mockClientBuilder, uri, "username", "password"); - fail("Expected an IllegalArgumentException."); - } catch (IllegalArgumentException e) { - assertEquals("Wrong error message was thrown", "java.net.MalformedURLException: unknown protocol: localhost", e.getMessage()); - } + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("java.net.MalformedURLException: unknown protocol: localhost"); + new ElasticSearchDao(mockClientBuilder, uri, "username", "password"); } @Test