From f2a2f208d1a4df162526e142c62c6802625306a1 Mon Sep 17 00:00:00 2001 From: Strongest Number 9 <16169054+StrongestNumber9@users.noreply.github.com> Date: Mon, 24 Jul 2023 15:23:14 +0300 Subject: [PATCH] Check for nulls for timestamps and logfiles , validate config and event formats (#61) * Should fix potential Null pointer dereference when parsing timestamps * Fixes situations where logfile might be null * Move logfile null check to more correct location * Move validation to the config object * Adds full config validation for configs * Try/catch Instant.parse instead of nullcheck * Adds validator for appname and hostname while handling messages * appname -> appName refactor * Adds missing exception constructors * Change all ints to integeres in AppConfig objects, check for null where necessary --- example/combined.yaml | 5 +- example/config/k8s_01/config.json | 1 + .../k8s_01/InvalidConfigurationException.java | 36 +++++++ .../java/com/teragrep/k8s_01/K8SConsumer.java | 100 ++++++++++++++++-- .../teragrep/k8s_01/KubernetesLogReader.java | 12 ++- .../com/teragrep/k8s_01/config/AppConfig.java | 31 ++++-- .../k8s_01/config/AppConfigKubernetes.java | 45 +++++++- .../k8s_01/config/AppConfigLabel.java | 22 +++- .../k8s_01/config/AppConfigLabels.java | 22 +++- .../k8s_01/config/AppConfigMetrics.java | 18 +++- .../teragrep/k8s_01/config/AppConfigRelp.java | 69 +++++++++--- .../teragrep/k8s_01/config/BaseConfig.java | 24 +++++ 12 files changed, 337 insertions(+), 48 deletions(-) create mode 100644 src/main/java/com/teragrep/k8s_01/InvalidConfigurationException.java create mode 100644 src/main/java/com/teragrep/k8s_01/config/BaseConfig.java diff --git a/example/combined.yaml b/example/combined.yaml index 825a8bf..8b69d6c 100644 --- a/example/combined.yaml +++ b/example/combined.yaml @@ -42,6 +42,7 @@ data: "kubernetes": { "logdir": "/var/log/containers", "url": "https://127.0.0.1:8443", + "timezone": "Europe/Helsinki", "cacheExpireInterval": 300, "cacheMaxEntries": 4096, "labels": { @@ -104,7 +105,7 @@ data: kind: ConfigMap metadata: - name: app-config-2t5785hm6f + name: app-config-58dh7f727h --- apiVersion: v1 data: @@ -271,7 +272,7 @@ spec: terminationGracePeriodSeconds: 0 volumes: - configMap: - name: app-config-2t5785hm6f + name: app-config-58dh7f727h name: app-config - hostPath: path: /var/log/containers diff --git a/example/config/k8s_01/config.json b/example/config/k8s_01/config.json index 6ef078e..c5e1afd 100644 --- a/example/config/k8s_01/config.json +++ b/example/config/k8s_01/config.json @@ -5,6 +5,7 @@ "kubernetes": { "logdir": "/var/log/containers", "url": "https://127.0.0.1:8443", + "timezone": "Europe/Helsinki", "cacheExpireInterval": 300, "cacheMaxEntries": 4096, "labels": { diff --git a/src/main/java/com/teragrep/k8s_01/InvalidConfigurationException.java b/src/main/java/com/teragrep/k8s_01/InvalidConfigurationException.java new file mode 100644 index 0000000..5b73a6c --- /dev/null +++ b/src/main/java/com/teragrep/k8s_01/InvalidConfigurationException.java @@ -0,0 +1,36 @@ +/* + Kubernetes log forwarder k8s_01 + Copyright (C) 2023 Suomen Kanuuna Oy + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package com.teragrep.k8s_01; + +public class InvalidConfigurationException extends Exception { + public InvalidConfigurationException() { + super(); + } + + public InvalidConfigurationException(String message) { + super(message); + } + + public InvalidConfigurationException(String message, Throwable throwable) { + super(message, throwable); + } + + public InvalidConfigurationException(Throwable throwable) { + super(throwable); + } +} diff --git a/src/main/java/com/teragrep/k8s_01/K8SConsumer.java b/src/main/java/com/teragrep/k8s_01/K8SConsumer.java index 2425728..19a6b44 100644 --- a/src/main/java/com/teragrep/k8s_01/K8SConsumer.java +++ b/src/main/java/com/teragrep/k8s_01/K8SConsumer.java @@ -36,9 +36,11 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; import java.util.UUID; import java.util.concurrent.BlockingQueue; import java.util.function.Consumer; +import java.util.regex.Pattern; /** * Must be thread-safe @@ -53,6 +55,10 @@ public class K8SConsumer implements Consumer { private static final DateTimeFormatter format = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSSSSxxx"); private final ZoneId timezoneId; + // Validators + private static final Pattern hostnamePattern = Pattern.compile("^[a-zA-Z0-9.-]+$"); // Not perfect but filters basically all mistakes + private static final Pattern appNamePattern = Pattern.compile("^[\\x21-\\x7e]+$"); // DEC 33 - DEC 126 as specified in RFC5424 + K8SConsumer( AppConfig appConfig, KubernetesCachingAPIClient cacheClient, @@ -131,7 +137,26 @@ public void accept(FileRecord record) { ) ); } - Instant instant = Instant.parse(log.getTimestamp()); + Instant instant; + try { + instant = Instant.parse(log.getTimestamp()); + } + catch(DateTimeParseException e) { + throw new RuntimeException( + String.format( + "[%s] Can't parse timestamp <%s> properly for event from pod <%s/%s> on container <%s> in file %s/%s at offset %s: ", + uuid, + log.getTimestamp(), + namespace, + podname, + containerId, + record.getPath(), + record.getFilename(), + record.getStartOffset() + ), + e + ); + } ZonedDateTime zdt = instant.atZone(timezoneId); String timestamp = zdt.format(format); @@ -152,26 +177,79 @@ public void accept(FileRecord record) { JsonObject dockerMetadata = new JsonObject(); dockerMetadata.addProperty("container_id", containerId); - // Handle hostname and appname, use fallback values when labels are empty or if label not found + // Handle hostname and appName, use fallback values when labels are empty or if label not found String hostname; - String appname; + String appName; if(podMetadataContainer.getLabels() == null) { LOGGER.warn( - "[{}] Can't resolve metadata and/or labels for container <{}>, using fallback values for hostname and appname", + "[{}] Can't resolve metadata and/or labels for container <{}>, using fallback values for hostname and appName", uuid, containerId ); hostname = appConfig.getKubernetes().getLabels().getHostname().getFallback(); - appname = appConfig.getKubernetes().getLabels().getAppname().getFallback(); + appName = appConfig.getKubernetes().getLabels().getAppName().getFallback(); } else { hostname = podMetadataContainer.getLabels().getOrDefault( appConfig.getKubernetes().getLabels().getHostname().getLabel(log.getStream()), appConfig.getKubernetes().getLabels().getHostname().getFallback() ); - appname = podMetadataContainer.getLabels().getOrDefault( - appConfig.getKubernetes().getLabels().getAppname().getLabel(log.getStream()), - appConfig.getKubernetes().getLabels().getAppname().getFallback() + appName = podMetadataContainer.getLabels().getOrDefault( + appConfig.getKubernetes().getLabels().getAppName().getLabel(log.getStream()), + appConfig.getKubernetes().getLabels().getAppName().getFallback() + ); + } + hostname = appConfig.getKubernetes().getLabels().getHostname().getPrefix() + hostname; + appName = appConfig.getKubernetes().getLabels().getAppName().getPrefix() + appName; + + if(!hostnamePattern.matcher(hostname).matches()) { + throw new RuntimeException( + String.format( + "[%s] Detected hostname <[%s]> from pod <[%s]/[%s]> on container <%s> contains invalid characters, can't continue", + uuid, + hostname, + namespace, + podname, + containerId + ) + ); + } + + if(hostname.length() >= 255) { + throw new RuntimeException( + String.format( + "[%s] Detected hostname <[%s]...> from pod <[%s]/[%s]> on container <%s> is too long, can't continue", + uuid, + hostname.substring(0,30), + namespace, + podname, + containerId + ) + ); + } + + if(!appNamePattern.matcher(appName).matches()) { + throw new RuntimeException( + String.format( + "[%s] Detected appName <[%s]> from pod <[%s]/[%s]> on container <%s> contains invalid characters, can't continue", + uuid, + appName, + namespace, + podname, + containerId + ) + ); + } + if(appName.length() > 48) { + throw new RuntimeException( + String.format( + "[%s] Detected appName <[%s]...> from pod <[%s]/[%s]> on container <%s> is too long, can't continue", + uuid, + appName.substring(0,30), + namespace, + podname, + containerId + ) ); } @@ -179,7 +257,7 @@ public void accept(FileRecord record) { LOGGER.debug( "[{}] Resolved message to be {}@{} from {}/{} generated at {}", uuid, - appname, + appName, hostname, namespace, podname, @@ -205,8 +283,8 @@ public void accept(FileRecord record) { SyslogMessage syslog = new SyslogMessage() .withTimestamp(timestamp, true) .withSeverity(Severity.WARNING) - .withHostname(appConfig.getKubernetes().getLabels().getHostname().getPrefix() + hostname) - .withAppName(appConfig.getKubernetes().getLabels().getAppname().getPrefix() + appname) + .withHostname(hostname) + .withAppName(appName) .withFacility(Facility.USER) .withSDElement(SDMetadata) .withMsg(new String(record.getRecord())); diff --git a/src/main/java/com/teragrep/k8s_01/KubernetesLogReader.java b/src/main/java/com/teragrep/k8s_01/KubernetesLogReader.java index acb36be..5e10334 100644 --- a/src/main/java/com/teragrep/k8s_01/KubernetesLogReader.java +++ b/src/main/java/com/teragrep/k8s_01/KubernetesLogReader.java @@ -42,6 +42,7 @@ public static void main(String[] args) throws IOException { AppConfig appConfig; try { appConfig = gson.fromJson(new FileReader("etc/config.json"), AppConfig.class); + appConfig.validate(); } catch (FileNotFoundException e) { LOGGER.error( @@ -57,6 +58,13 @@ public static void main(String[] args) throws IOException { ); return; } + catch (InvalidConfigurationException e) { + LOGGER.error( + "Failed to validate config 'etc/config.json':", + e + ); + return; + } catch (Exception e) { LOGGER.error( "Unknown exception while handling config:", @@ -108,23 +116,25 @@ public static void main(String[] args) throws IOException { // consumer supplier, returns always the same instance K8SConsumerSupplier consumerSupplier = new K8SConsumerSupplier(appConfig, cacheClient, relpOutputPool); - String[] logfiles = appConfig.getKubernetes().getLogfiles(); LOGGER.debug( "Monitored logfiles: {}", Arrays.toString(logfiles) ); + List threads = new ArrayList<>(); String statesStore = System.getProperty("user.dir") + "/var"; LOGGER.debug( "Using {} as statestore", statesStore ); + // FIXME: VERIFY: SFR is not in try-with-resources block as it will have weird behaviour with threads. StatefulFileReader statefulFileReader = new StatefulFileReader( Paths.get(statesStore), consumerSupplier ); + // Start a new thread for all logfile watchers for (String logfile : logfiles) { Thread thread = new Thread(() -> { diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfig.java b/src/main/java/com/teragrep/k8s_01/config/AppConfig.java index f55a192..ffc42ad 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfig.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfig.java @@ -18,22 +18,19 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; /* POJO representing the main config.json */ -public class AppConfig { - private AppConfigKubernetes kubernetes; - +public class AppConfig implements BaseConfig { + private AppConfigMetrics metrics; public AppConfigMetrics getMetrics() { return metrics; } - - private AppConfigMetrics metrics; - private AppConfigRelp relp; - + private AppConfigKubernetes kubernetes; public AppConfigKubernetes getKubernetes() { return kubernetes; } - + private AppConfigRelp relp; public AppConfigRelp getRelp() { return relp; } @@ -42,4 +39,22 @@ public AppConfigRelp getRelp() { public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if(metrics == null) { + throw new InvalidConfigurationException("metrics object not found or is null in main config object"); + } + getMetrics().validate(); + + if(kubernetes == null) { + throw new InvalidConfigurationException("kubernetes object not found or is null in main config object"); + } + getKubernetes().validate(); + + if(relp == null) { + throw new InvalidConfigurationException("relp object no found or is null in main config object"); + } + getRelp().validate(); + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfigKubernetes.java b/src/main/java/com/teragrep/k8s_01/config/AppConfigKubernetes.java index 0c7dc6d..4aff844 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfigKubernetes.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfigKubernetes.java @@ -18,18 +18,19 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; /* POJO representing the .kubernetes part of config.json */ -public class AppConfigKubernetes { - private int cacheExpireInterval; - private int cacheMaxEntries; +public class AppConfigKubernetes implements BaseConfig { + private Integer cacheExpireInterval; + private Integer cacheMaxEntries; private String logdir; private AppConfigLabels labels; private String[] logfiles; private String url; private String timezone; - public int getCacheExpireInterval() { + public Integer getCacheExpireInterval() { return cacheExpireInterval; } @@ -48,7 +49,7 @@ public String[] getLogfiles() { public String getUrl() { return url; } - public int getCacheMaxEntries() { + public Integer getCacheMaxEntries() { return cacheMaxEntries; } public String getTimezone() { return timezone; } @@ -57,4 +58,38 @@ public int getCacheMaxEntries() { public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if(cacheExpireInterval == null) { + throw new InvalidConfigurationException("cacheExpireInterval not found or is null in kubernetes config object"); + } + if(cacheMaxEntries == null) { + throw new InvalidConfigurationException("cacheMaxEntries not found or is null in kubernetes config object"); + } + if(logdir == null) { + throw new InvalidConfigurationException("logdir not found or is null in kubernetes config object"); + } + if(labels == null) { + throw new InvalidConfigurationException("labels not found or is null in kubernetes config object"); + } + labels.validate(); + + if(logfiles == null) { + throw new InvalidConfigurationException("logfiles not found or is null in kubernetes config object"); + } + for (String logfile : logfiles) { + if (logfile == null) { + throw new InvalidConfigurationException("Found null logfile definition in configuration file, expected string"); + } + } + + if(url == null) { + throw new InvalidConfigurationException("url not found or is null in kubernetes config object"); + } + + if(timezone == null) { + throw new InvalidConfigurationException("timezone not found or is null in kubernetes config object"); + } + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfigLabel.java b/src/main/java/com/teragrep/k8s_01/config/AppConfigLabel.java index 48daa5f..1dca56a 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfigLabel.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfigLabel.java @@ -18,13 +18,16 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; + +import java.util.regex.Pattern; /* POJO representing the .kubernetes.labels.{hostname,appname} part of config.json */ -public class AppConfigLabel { +public class AppConfigLabel implements BaseConfig { private String prefix; private String fallback; private String labelStdout; - private String labelStderr; + private String labelStderr; // Can be null, is handled in getLabel public String getPrefix() { return prefix; @@ -45,4 +48,19 @@ public String getLabel(String label) { public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if(prefix == null) { + throw new InvalidConfigurationException("prefix not found or is null in label config object"); + } + + if(fallback == null) { + throw new InvalidConfigurationException("fallback not found or is null in label config object"); + } + + if(labelStdout == null) { + throw new InvalidConfigurationException("labelStdout not found or is null in label config object"); + } + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfigLabels.java b/src/main/java/com/teragrep/k8s_01/config/AppConfigLabels.java index 82da7cc..89566f2 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfigLabels.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfigLabels.java @@ -18,17 +18,20 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; + +import java.util.regex.Pattern; /* POJO representing the .kubernetes.labels part of config.json */ -public class AppConfigLabels { +public class AppConfigLabels implements BaseConfig { private AppConfigLabel hostname; - private AppConfigLabel appname; + private AppConfigLabel appname; // Lowercase instead of appName because it comes from json and needs to be case-sensitive public AppConfigLabel getHostname() { return hostname; } - public AppConfigLabel getAppname() { + public AppConfigLabel getAppName() { return appname; } @@ -36,4 +39,17 @@ public AppConfigLabel getAppname() { public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if(hostname == null) { + throw new InvalidConfigurationException("hostname not found or is null in labels config object"); + } + hostname.validate(); + + if(appname == null) { + throw new InvalidConfigurationException("appname not found or is null in labels config object"); + } + appname.validate(); + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfigMetrics.java b/src/main/java/com/teragrep/k8s_01/config/AppConfigMetrics.java index 1b80966..2e4551d 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfigMetrics.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfigMetrics.java @@ -18,16 +18,28 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; -public class AppConfigMetrics { - public int getPort() { +public class AppConfigMetrics implements BaseConfig { + public Integer getPort() { return port; } - private int port; + private Integer port; @Override public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if (port == null) { + throw new InvalidConfigurationException("port not found or is null in metrics config object"); + } + + if(port < 1 || port > 65535) { + throw new InvalidConfigurationException("Metrics port is invalid, expected integer between 1 and 65535"); + } + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/AppConfigRelp.java b/src/main/java/com/teragrep/k8s_01/config/AppConfigRelp.java index cab41a9..45679b8 100644 --- a/src/main/java/com/teragrep/k8s_01/config/AppConfigRelp.java +++ b/src/main/java/com/teragrep/k8s_01/config/AppConfigRelp.java @@ -18,42 +18,43 @@ package com.teragrep.k8s_01.config; import com.google.gson.Gson; +import com.teragrep.k8s_01.InvalidConfigurationException; /* POJO representing the .relp part of config.json */ -public class AppConfigRelp { +public class AppConfigRelp implements BaseConfig { private String target; - private int port; - private int connectionTimeout; - private int readTimeout; - private int writeTimeout; - private int reconnectInterval; - private int outputThreads; + private Integer port; + private Integer connectionTimeout; + private Integer readTimeout; + private Integer writeTimeout; + private Integer reconnectInterval; + private Integer outputThreads; public String getTarget() { return target; } - public int getPort() { + public Integer getPort() { return port; } - public int getConnectionTimeout() { + public Integer getConnectionTimeout() { return connectionTimeout; } - public int getReadTimeout() { + public Integer getReadTimeout() { return readTimeout; } - public int getWriteTimeout() { + public Integer getWriteTimeout() { return writeTimeout; } - public int getReconnectInterval() { + public Integer getReconnectInterval() { return reconnectInterval; } - public int getOutputThreads() { + public Integer getOutputThreads() { return outputThreads; } @@ -61,4 +62,46 @@ public int getOutputThreads() { public String toString() { return new Gson().toJson(this); } + + @Override + public void validate() throws InvalidConfigurationException { + if(target == null) { + throw new InvalidConfigurationException("target not found or is null in relp config object"); + } + + if(port == null) { + throw new InvalidConfigurationException("port not found or is null in relp config object"); + } + if(port < 1 || port > 65535) { + throw new InvalidConfigurationException("Relp port is invalid, expected integer between 1 and 65535"); + } + + if(connectionTimeout == null) { + throw new InvalidConfigurationException("connectionTimeout not found or is null in relp config object"); + } + if(connectionTimeout < 0) { + throw new InvalidConfigurationException("Relp connection timeout is invalid, expected positive integer"); + } + + if(readTimeout == null) { + throw new InvalidConfigurationException("readTimeout not found or is null in relp config object"); + } + if(readTimeout < 0) { + throw new InvalidConfigurationException("Relp read timeout is invalid, expected positive integer"); + } + + if(writeTimeout == null) { + throw new InvalidConfigurationException("writeTimeout not found or is null in relp config object"); + } + if(writeTimeout < 0) { + throw new InvalidConfigurationException("Relp write timeout is invalid, expected positive integer"); + } + + if(outputThreads == null) { + throw new InvalidConfigurationException("outputThreads not found or is null in relp config object"); + } + if(outputThreads <= 0) { + throw new InvalidConfigurationException("Relp output threads is invalid, expected >0"); + } + } } diff --git a/src/main/java/com/teragrep/k8s_01/config/BaseConfig.java b/src/main/java/com/teragrep/k8s_01/config/BaseConfig.java new file mode 100644 index 0000000..514bae8 --- /dev/null +++ b/src/main/java/com/teragrep/k8s_01/config/BaseConfig.java @@ -0,0 +1,24 @@ +/* + Kubernetes log forwarder k8s_01 + Copyright (C) 2023 Suomen Kanuuna Oy + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package com.teragrep.k8s_01.config; + +import com.teragrep.k8s_01.InvalidConfigurationException; + +public interface BaseConfig { + public void validate() throws InvalidConfigurationException; +}