From f22065d54e715474477dc0afef9f6e81a27bd66e Mon Sep 17 00:00:00 2001 From: Stephan Fuhrmann Date: Mon, 8 Jan 2024 15:39:46 +0100 Subject: [PATCH] Move walk reading from the SnmpmanAgent to new Walks class Add unit test for the Walks class Allow Hex-STRING to be multi-line --- .../com/oneandone/snmpman/SnmpmanAgent.java | 113 ++--------- .../snmpman/configuration/Walks.java | 182 ++++++++++++++++++ .../snmpman/configuration/WalksTest.java | 87 +++++++++ 3 files changed, 280 insertions(+), 102 deletions(-) create mode 100644 snmpman/src/main/java/com/oneandone/snmpman/configuration/Walks.java create mode 100644 snmpman/src/test/java/com/oneandone/snmpman/configuration/WalksTest.java diff --git a/snmpman/src/main/java/com/oneandone/snmpman/SnmpmanAgent.java b/snmpman/src/main/java/com/oneandone/snmpman/SnmpmanAgent.java index 4f19775..d5f00d6 100644 --- a/snmpman/src/main/java/com/oneandone/snmpman/SnmpmanAgent.java +++ b/snmpman/src/main/java/com/oneandone/snmpman/SnmpmanAgent.java @@ -3,6 +3,7 @@ import com.google.common.primitives.UnsignedLong; import com.oneandone.snmpman.configuration.AgentConfiguration; import com.oneandone.snmpman.configuration.Device; +import com.oneandone.snmpman.configuration.Walks; import com.oneandone.snmpman.configuration.modifier.CommunityContextModifier; import com.oneandone.snmpman.configuration.modifier.ModifiedVariable; import com.oneandone.snmpman.configuration.modifier.Modifier; @@ -46,11 +47,6 @@ public class SnmpmanAgent extends BaseAgent { */ private static final Charset DEFAULT_CHARSET = Charset.forName("UTF-8"); - /** - * The pattern of variable bindings in a walk file. - */ - private static final Pattern VARIABLE_BINDING_PATTERN = Pattern.compile("(((iso)?\\.[0-9]+)+) = ((([a-zA-Z0-9-]+): (.*)$)|(\"\"$))"); - /** * The configuration of this agent. */ @@ -161,53 +157,6 @@ private static List getRoots(final SortedMap bindings) { return roots; } - /** - * Returns a {@link Variable} instance for the specified parameters. - * - * @param type the type of the variable - * @param value the value of this variable - * @return a a {@link Variable} instance with the specified type and value - * @throws IllegalArgumentException if the type could not be mapped to a {@link Variable} implementation - */ - private static Variable getVariable(final String type, final String value) { - switch (type) { - // TODO add "BITS" support - case "STRING": - if (value.startsWith("\"") && value.endsWith("\"")) { - if (value.length() == 2) { - return new OctetString(); - } - return new OctetString(value.substring(1, value.length() - 1)); - } else { - return new OctetString(value); - } - case "OID": - return new OID(value); - case "Gauge32": - return new Gauge32(Long.parseLong(value.replaceAll("[^-?0-9]+", ""))); - case "Timeticks": - final int openBracket = value.indexOf("(") + 1; - final int closeBracket = value.indexOf(")"); - if (openBracket == 0 || closeBracket < 0) { - throw new IllegalArgumentException("could not parse time tick value in " + value); - } - return new TimeTicks(Long.parseLong(value.substring(openBracket, closeBracket))); - case "Counter32": - return new Counter32(Long.parseLong(value.replaceAll("[^-?0-9]+", ""))); - case "Counter64": - // Parse unsigned long - return new Counter64(UnsignedLong.valueOf(value).longValue()); - case "INTEGER": - return new Integer32(Integer.parseInt(value.replaceAll("[^-?0-9]+", ""))); - case "Hex-STRING": - return OctetString.fromHexString(value, ' '); - case "IpAddress": - return new IpAddress(value); - default: - throw new IllegalArgumentException("illegal type \"" + type + "\" in walk detected"); - } - } - /** * Starts this agent instance. * @@ -249,10 +198,8 @@ protected void registerManagedObjects() { log.trace("registering managed objects for agent \"{}\"", configuration.getName()); for (final Long vlan : vlans) { - try (final FileInputStream fileInputStream = new FileInputStream(configuration.getWalk()); - final BufferedReader reader = new BufferedReader(new InputStreamReader(fileInputStream, DEFAULT_CHARSET))) { - - Map bindings = readVariableBindings(reader); + try { + Map bindings = Walks.readWalk(configuration.getWalk()); final SortedMap variableBindings = this.getVariableBindings(configuration.getDevice(), bindings, new OctetString(String.valueOf(vlan))); @@ -281,10 +228,9 @@ protected void registerManagedObjects() { registerGroupAndContext(group, context); } } - } catch (final FileNotFoundException e) { - log.error("walk file {} not found", configuration.getWalk().getAbsolutePath()); - } catch (final IOException e) { - log.error("could not read walk file " + configuration.getWalk().getAbsolutePath(), e); + } + catch (IOException e) { + log.error("Could not read walk file " + configuration.getWalk().getAbsolutePath(), e); } } createAndRegisterDefaultContext(); @@ -304,20 +250,17 @@ private MOGroup createGroup(final OID root, final SortedMap varia * Creates the {@link StaticMOGroup} with all information necessary to register it to the server. */ private void createAndRegisterDefaultContext() { - try (final FileInputStream fileInputStream = new FileInputStream(configuration.getWalk()); - final BufferedReader reader = new BufferedReader(new InputStreamReader(fileInputStream, DEFAULT_CHARSET))) { - - Map bindings = readVariableBindings(reader); + try { + Map bindings = Walks.readWalk(configuration.getWalk()); final SortedMap variableBindings = this.getVariableBindings(configuration.getDevice(), bindings, new OctetString()); final List roots = SnmpmanAgent.getRoots(variableBindings); for (final OID root : roots) { MOGroup group = createGroup(root, variableBindings); registerDefaultGroups(group); } - } catch (final FileNotFoundException e) { - log.error("walk file {} not found", configuration.getWalk().getAbsolutePath()); - } catch (final IOException e) { - log.error("could not read walk file " + configuration.getWalk().getAbsolutePath(), e); + } + catch (IOException e) { + log.error("Could not read walk file " + configuration.getWalk().getAbsolutePath(), e); } } @@ -395,40 +338,6 @@ private void registerHard(final MOGroup group) { } } - /** - * Reads all variable bindings using {@link #VARIABLE_BINDING_PATTERN}. - * - * @param reader the reader to read the bindings from. - * @return the map of oid to variable binding. - */ - private Map readVariableBindings(final BufferedReader reader) throws IOException { - final Map bindings = new HashMap<>(); - String line; - while ((line = reader.readLine()) != null) { - final Matcher matcher = VARIABLE_BINDING_PATTERN.matcher(line); - if (matcher.matches()) { - final OID oid = new OID(matcher.group(1).replace("iso", ".1")); - - try { - final Variable variable; - if (matcher.group(7) == null) { - variable = SnmpmanAgent.getVariable("STRING", "\"\""); - } else { - variable = SnmpmanAgent.getVariable(matcher.group(6), matcher.group(7)); - } - - bindings.put(oid, variable); - log.trace("added binding with oid \"{}\" and variable \"{}\"", oid, variable); - } catch (final Exception e) { - log.warn("could not parse line \"{}\" of walk file {} with exception: {}", line, configuration.getWalk().getCanonicalPath(), e.getMessage()); - } - } else { - log.warn("could not parse line \"{}\" of walk file {}", line, configuration.getWalk().getAbsolutePath()); - } - } - return bindings; - } - /** * Unregisters all default managed objects in the specified context {@code ctx}. * diff --git a/snmpman/src/main/java/com/oneandone/snmpman/configuration/Walks.java b/snmpman/src/main/java/com/oneandone/snmpman/configuration/Walks.java new file mode 100644 index 0000000..3e79f3c --- /dev/null +++ b/snmpman/src/main/java/com/oneandone/snmpman/configuration/Walks.java @@ -0,0 +1,182 @@ +package com.oneandone.snmpman.configuration; + +import com.google.common.primitives.UnsignedLong; +import com.oneandone.snmpman.SnmpmanAgent; +import lombok.extern.slf4j.Slf4j; +import org.snmp4j.smi.Counter32; +import org.snmp4j.smi.Counter64; +import org.snmp4j.smi.Gauge32; +import org.snmp4j.smi.Integer32; +import org.snmp4j.smi.IpAddress; +import org.snmp4j.smi.OID; +import org.snmp4j.smi.OctetString; +import org.snmp4j.smi.TimeTicks; +import org.snmp4j.smi.Variable; + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** Helper class for reading SNMP walks. + * */ +@Slf4j +public class Walks { + + /** + * The default charset for files being read. + */ + private static final Charset DEFAULT_CHARSET = Charset.forName("UTF-8"); + + /** + * The pattern of variable bindings in a walk file. + */ + private static final Pattern VARIABLE_BINDING_PATTERN = Pattern.compile("(((iso)?\\.[0-9]+)+) = ((([a-zA-Z0-9-]+): (.*)$)|(\"\"$))"); + + /** + * The pattern of a Hex-STRING continuation line in a walk file. + */ + private static final Pattern HEX_STRING_PATTERN = Pattern.compile("([0-9a-fA-F]{2})( [0-9a-fA-F]{2})+"); + + private Walks() { + + } + + /** Reads a walk from a file. + * @param walk the walk file to read. + * @return the map of oid to variable binding from the file. + * @throws IOException if the file could not be read. + * */ + public static Map readWalk(final File walk) throws IOException { + log.debug("Reading walk from file {}", walk); + try (final FileInputStream fileInputStream = new FileInputStream(walk); + final BufferedReader reader = new BufferedReader(new InputStreamReader(fileInputStream, DEFAULT_CHARSET))) { + Map result = readVariableBindings(walk, reader); + log.debug("Walk contains {} variable bindings", result.size()); + return result; + } + catch (final FileNotFoundException e) { + log.error("walk file {} not found", walk.getAbsolutePath()); + throw e; + } catch (final IOException e) { + log.error("could not read walk file " + walk.getAbsolutePath(), e); + throw e; + } + } + + /** + * Reads all variable bindings using {@link #VARIABLE_BINDING_PATTERN}. + * + * @param reader the reader to read the bindings from. + * @return the map of oid to variable binding. + */ + static Map readVariableBindings(final File walk, final BufferedReader reader) throws IOException { + final Map bindings = new HashMap<>(); + OID lastOid = null; + String lastType = null; + String line; + while ((line = reader.readLine()) != null) { + boolean match = false; + Matcher matcher = VARIABLE_BINDING_PATTERN.matcher(line); + if (matcher.matches()) { + match = true; + final OID oid = new OID(matcher.group(1).replace("iso", ".1")); + lastOid = oid; + + try { + final Variable variable; + if (matcher.group(7) == null) { + lastType = "STRING"; + variable = getVariable("STRING", "\"\""); + } else { + lastType = matcher.group(6); + variable = getVariable(lastType, matcher.group(7)); + } + + bindings.put(oid, variable); + log.trace("added binding with oid \"{}\" and variable \"{}\"", oid, variable); + } catch (final Exception e) { + log.warn("could not parse line \"{}\" of walk file {} with exception: {}", line, walk.getCanonicalPath(), e.getMessage()); + } + } + + // if we have a continuation line for a Hex-STRING, append to it + if (lastType != null && lastOid != null && lastType.equals("Hex-STRING")) { + matcher = HEX_STRING_PATTERN.matcher(line); + if (matcher.matches()) { + OctetString octetStringToExtend = (OctetString) bindings.get(lastOid); + if (octetStringToExtend != null) { + byte[] oldBytes = octetStringToExtend.getValue(); + byte[] newBytes = OctetString.fromHexString(matcher.group(0), ' ').toByteArray(); + byte[] combined = new byte[oldBytes.length + newBytes.length]; + System.arraycopy(oldBytes, 0, combined, 0, oldBytes.length); + System.arraycopy(newBytes, 0, combined, oldBytes.length, newBytes.length); + bindings.put(lastOid, new OctetString(combined)); + } else { + log.warn("Could not find the previous octet string of OID {} in walk file {}", lastOid); + } + } + } + + if (!match) { + log.warn("could not parse line \"{}\" of walk file {}", line, walk.getAbsolutePath()); + } + } + return bindings; + } + + /** + * Returns a {@link Variable} instance for the specified parameters. + * + * @param type the type of the variable + * @param value the value of this variable + * @return a a {@link Variable} instance with the specified type and value + * @throws IllegalArgumentException if the type could not be mapped to a {@link Variable} implementation + */ + private static Variable getVariable(final String type, final String value) { + switch (type) { + // TODO add "BITS" support + case "STRING": + if (value.startsWith("\"") && value.endsWith("\"")) { + if (value.length() == 2) { + return new OctetString(); + } + return new OctetString(value.substring(1, value.length() - 1)); + } else { + return new OctetString(value); + } + case "OID": + return new OID(value); + case "Gauge32": + return new Gauge32(Long.parseLong(value.replaceAll("[^-?0-9]+", ""))); + case "Timeticks": + final int openBracket = value.indexOf("(") + 1; + final int closeBracket = value.indexOf(")"); + if (openBracket == 0 || closeBracket < 0) { + throw new IllegalArgumentException("could not parse time tick value in " + value); + } + return new TimeTicks(Long.parseLong(value.substring(openBracket, closeBracket))); + case "Counter32": + return new Counter32(Long.parseLong(value.replaceAll("[^-?0-9]+", ""))); + case "Counter64": + // Parse unsigned long + return new Counter64(UnsignedLong.valueOf(value).longValue()); + case "INTEGER": + return new Integer32(Integer.parseInt(value.replaceAll("[^-?0-9]+", ""))); + case "Hex-STRING": + return OctetString.fromHexString(value, ' '); + case "IpAddress": + return new IpAddress(value); + default: + throw new IllegalArgumentException("illegal type \"" + type + "\" in walk detected"); + } + } +} diff --git a/snmpman/src/test/java/com/oneandone/snmpman/configuration/WalksTest.java b/snmpman/src/test/java/com/oneandone/snmpman/configuration/WalksTest.java new file mode 100644 index 0000000..c1f4575 --- /dev/null +++ b/snmpman/src/test/java/com/oneandone/snmpman/configuration/WalksTest.java @@ -0,0 +1,87 @@ +package com.oneandone.snmpman.configuration; + +import com.oneandone.snmpman.configuration.modifier.Counter32Modifier; +import com.oneandone.snmpman.configuration.modifier.Modifier; +import com.oneandone.snmpman.configuration.type.ModifierProperties; +import org.snmp4j.smi.OID; +import org.snmp4j.smi.OctetString; +import org.snmp4j.smi.Variable; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +public class WalksTest { + private Path tmpFile; + + @BeforeMethod + public void setUp() throws IOException { + tmpFile = Files.createTempFile("snmpman", "walk"); + } + + @AfterMethod + public void tearDown() { + if (tmpFile != null) { + tmpFile.toFile().delete(); + } + } + + @Test + public void readWalkWithEmptyFile() throws IOException { + Map walk = Walks.readWalk(tmpFile.toFile()); + assertEquals(walk, Collections.emptyMap()); + } + + @Test + public void readWalkWithStringLine() throws IOException { + Files.write(tmpFile, Collections.singletonList(".1.3.6.1.2.1.2.2.1.2.10101 = STRING: \"GigabitEthernet0/1\"")); + Map walk = Walks.readWalk(tmpFile.toFile()); + assertEquals(walk, Collections.singletonMap( + new OID(".1.3.6.1.2.1.2.2.1.2.10101"), + new OctetString("GigabitEthernet0/1"))); + } + + @Test + public void readWalkWithHexStringLine() throws IOException { + Files.write(tmpFile, Collections.singletonList(".1.3.6.1.4.1.9.9.683.1.5.0 = Hex-STRING: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f")); + Map walk = Walks.readWalk(tmpFile.toFile()); + byte[] expected = new byte[16]; + for (int i = 0; i < 16; i++) { + expected[i] = (byte) i; + } + assertEquals(walk, Collections.singletonMap( + new OID(".1.3.6.1.4.1.9.9.683.1.5.0"), + new OctetString(expected))); + } + + @Test + public void readWalkWithTwoHexStringLine() throws IOException { + Files.write(tmpFile, Arrays.asList( + ".1.3.6.1.4.1.9.9.683.1.5.0 = Hex-STRING: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f", + "10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f")); + Map walk = Walks.readWalk(tmpFile.toFile()); + byte[] expected = new byte[32]; + for (int i = 0; i < 32; i++) { + expected[i] = (byte) i; + } + assertEquals(walk, Collections.singletonMap( + new OID(".1.3.6.1.4.1.9.9.683.1.5.0"), + new OctetString(expected))); + } + + @Test + public void readWalkWithInvalidLine() throws IOException { + Files.write(tmpFile, Collections.singletonList("this is just an example")); + Map walk = Walks.readWalk(tmpFile.toFile()); + assertEquals(walk, Collections.emptyMap()); + } +}