Refactor configuration of ClusterLogForwarder

With this commit configuring the ClusterLogForwarder resource will be done with
a new parameter `openshift4_logging.clusterLogForwarder`, which will follow the
upstream convention / spec.

The old parameter `openshift4_logging.clusterLogForwarding` is deprecated, but
existing legacy config will be respected.

class/defaults.yml

@@ -71,20 +71,7 @@ parameters:
               memory: 128Mi
 
     clusterLogging: {}
-
-    clusterLogForwarding:
-      enabled: false
-      forwarders: {}
-      namespace_groups: {}
-      application_logs: {}
-      audit_logs:
-        enabled: false
-      infrastructure_logs:
-        enabled: true
-      json:
-        enabled: false
-        typekey: 'kubernetes.labels.logFormat'
-        typename: 'nologformat'
+    clusterLogForwarder: {}
 
     operatorResources:
       clusterLogging:

component/config_forwarding.libsonnet

@@ -1,3 +1,4 @@
+local com = import 'lib/commodore.libjsonnet';
 local kap = import 'lib/kapitan.libjsonnet';
 local lib = import 'lib/openshift4-logging.libsonnet';
 
@@ -8,70 +9,84 @@ local deployLokistack = params.components.lokistack.enabled;
 local deployElasticsearch = params.components.elasticsearch.enabled;
 local forwardingOnly = !deployLokistack && !deployElasticsearch;
 
+// -----------------------------------------------------------------------------
+//   Legacy Rendering
+// -----------------------------------------------------------------------------
+
+local legacyConfigParams = std.get(params, 'clusterLogForwarding', {});
+local legacyConfig = if std.length(legacyConfigParams) > 0 then std.trace(
+  'Parameter `clusterLogForwarding` is deprecated. Please update your config to use `clusterLogForwarder`',
+  legacyConfigParams
+) else {};
+
 local pipelineOutputRefs(pipeline) =
   local default = if forwardingOnly then [] else [ 'default' ];
   std.get(pipeline, 'forwarders', []) + default;
 
 // Apply default config for application logs.
-local patchAppLogDefaults = {
-  local outputRefs = pipelineOutputRefs(params.clusterLogForwarding.application_logs),
-  local enablePipeline = std.length(outputRefs) > 0,
+local patchLegacyAppLogDefaults = {
+  local pipeline = std.get(legacyConfig, 'application_logs', { enabled: true }),
+  local pipelineOutputs = pipelineOutputRefs(pipeline),
+  local pipelineEnabled = std.length(pipelineOutputs) > 0,
 
   pipelines: {
-    [if enablePipeline then 'application-logs']: {
+    [if pipelineEnabled then 'application-logs']: {
       inputRefs: [ 'application' ],
-      outputRefs: outputRefs,
+      outputRefs: pipelineOutputs,
     },
   },
 };
 
 // Apply default config for infra logs.
-local patchInfraLogDefaults = {
-  local outputRefs = pipelineOutputRefs(params.clusterLogForwarding.infrastructure_logs),
-  local enablePipeline = params.clusterLogForwarding.infrastructure_logs.enabled && std.length(outputRefs) > 0,
+local patchLegacyInfraLogDefaults = {
+  local pipeline = { enabled: true } + std.get(legacyConfig, 'infrastructure_logs', {}),
+  local pipelineOutputs = pipelineOutputRefs(pipeline),
+  local pipelineEnabled = pipeline.enabled && std.length(pipelineOutputs) > 0,
 
   pipelines: {
-    [if enablePipeline then 'infrastructure-logs']: {
+    [if pipelineEnabled then 'infrastructure-logs']: {
       inputRefs: [ 'infrastructure' ],
-      outputRefs: outputRefs,
+      outputRefs: pipelineOutputs,
     },
   },
 };
 
 // Apply default config for audit logs.
-local patchAuditLogDefaults = {
-  local outputRefs = pipelineOutputRefs(params.clusterLogForwarding.audit_logs),
-  local enablePipeline = params.clusterLogForwarding.audit_logs.enabled && std.length(outputRefs) > 0,
+local patchLegacyAuditLogDefaults = {
+  local pipeline = std.get(legacyConfig, 'audit_logs', { enabled: false }),
+  local pipelineOutputs = pipelineOutputRefs(pipeline),
+  local pipelineEnabled = pipeline.enabled && std.length(pipelineOutputs) > 0,
 
   pipelines: {
-    [if enablePipeline then 'audit-logs']: {
+    [if pipelineEnabled then 'audit-logs']: {
       inputRefs: [ 'audit' ],
-      outputRefs: outputRefs,
+      outputRefs: pipelineOutputs,
     },
   },
 };
 
 // Enable json parsing for default pipelines if configured.
-local patchJsonLogging = {
-  local enableAppLogs = std.get(params.clusterLogForwarding.application_logs, 'json', false),
-  local enableInfraLogs = std.get(params.clusterLogForwarding.infrastructure_logs, 'json', false),
+local legacyEnableJson = std.get(std.get(legacyConfig, 'json', {}), 'enabled', false);
+local patchLegacyJsonLogging = {
+  local enableAppLogs = std.get(std.get(legacyConfig, 'application_logs', {}), 'json', false),
+  local enableInfraLogs = std.get(std.get(legacyConfig, 'infrastructure_logs', {}), 'json', false),
 
   pipelines: {
     [if enableAppLogs then 'application-logs']: { parse: 'json' },
     [if enableInfraLogs then 'infrastructure-logs']: { parse: 'json' },
   },
-  [if deployElasticsearch && params.clusterLogForwarding.json.enabled then 'outputDefaults']: {
+  [if deployElasticsearch && legacyEnableJson then 'outputDefaults']: {
     elasticsearch: {
-      structuredTypeKey: params.clusterLogForwarding.json.typekey,
-      structuredTypeName: params.clusterLogForwarding.json.typename,
+      structuredTypeKey: std.get(legacyConfig.json, 'typekey', 'kubernetes.labels.logFormat'),
+      structuredTypeName: std.get(legacyConfig.json, 'typename', 'nologformat'),
     },
   },
 };
 
 // Enable detectMultilineErrors for default pipelines if configured.
-local patchMultilineErrors = {
-  local enableAppLogs = std.get(params.clusterLogForwarding.application_logs, 'detectMultilineErrors', false),
-  local enableInfraLogs = std.get(params.clusterLogForwarding.infrastructure_logs, 'detectMultilineErrors', false),
+local patchLegacyMultilineErrors = {
+  local enableAppLogs = std.get(std.get(legacyConfig, 'application_logs', {}), 'detectMultilineErrors', false),
+  local enableInfraLogs = std.get(std.get(legacyConfig, 'infrastructure_logs', {}), 'detectMultilineErrors', false),
 
   pipelines: {
     [if enableAppLogs then 'application-logs']: { detectMultilineErrors: true },
@@ -81,19 +96,19 @@ local patchMultilineErrors = {
 
 // --- patch deprecated `clusterLogForwarding.namespace` config
 local namespaceGroups = (
-  if std.objectHas(params.clusterLogForwarding, 'namespaces') then
+  if std.objectHas(legacyConfig, 'namespaces') then
     {
       [ns]: {
         namespaces: [ ns ],
-        forwarders: [ params.clusterLogForwarding.namespaces[ns].forwarder ],
+        forwarders: [ legacyConfig.namespaces[ns].forwarder ],
       }
-      for ns in std.objectFields(params.clusterLogForwarding.namespaces)
+      for ns in std.objectFields(legacyConfig.namespaces)
     } else {}
-) + params.clusterLogForwarding.namespace_groups;
+) + std.get(legacyConfig, 'namespace_groups', {});
 // --- patch end
 
 // Add inputs entry for every namespace_group defined in `clusterLogForwarding.namespace_groups`.
-local patchCustomInputs = {
+local patchLegacyCustomInputs = {
   [if std.length(namespaceGroups) > 0 then 'inputs']: {
     [group]: {
       application: {
@@ -105,7 +120,7 @@ local patchCustomInputs = {
 };
 
 // Add pipelines entry for every namespace_group defined in `clusterLogForwarding.namespace_groups`.
-local patchCustomPipelines = {
+local patchLegacyCustomPipelines = {
   [if std.length(namespaceGroups) > 0 then 'pipelines']: {
     local enableJson = std.get(namespaceGroups[group], 'json', false),
     local enableMultilineError = std.get(namespaceGroups[group], 'detectMultilineErrors', false),
@@ -121,35 +136,39 @@ local patchCustomPipelines = {
 };
 
 // Add outputs entry for every forwarder defined in `clusterLogForwarding.forwarders`.
-local patchCustomOutputs = {
-  [if std.length(params.clusterLogForwarding.forwarders) > 0 then 'outputs']: {
-    [name]: params.clusterLogForwarding.forwarders[name]
-    for name in std.objectFields(params.clusterLogForwarding.forwarders)
+local patchLegacyCustomOutputs = {
+  [if std.length(std.get(legacyConfig, 'forwarders', {})) > 0 then 'outputs']: {
+    [name]: legacyConfig.forwarders[name]
+    for name in std.objectFields(legacyConfig.forwarders)
   },
 };
 
-// ClusterLogForwarderSpecs:
+// -----------------------------------------------------------------------------
+//   End Legacy Rendering
+// -----------------------------------------------------------------------------
+
+// clusterLogForwarderSpec:
 // Consecutively apply patches to result of previous apply.
 local clusterLogForwarderSpec = std.foldl(
   // we use std.mergePatch here, because this way we don't need
   // to make each patch object mergeable by suffixing all keys with a +.
   function(manifest, patch) std.mergePatch(manifest, patch),
   [
-    patchAppLogDefaults,
-    patchInfraLogDefaults,
-    patchAuditLogDefaults,
-    patchJsonLogging,
-    patchMultilineErrors,
-    patchCustomInputs,
-    patchCustomOutputs,
-    patchCustomPipelines,
+    patchLegacyAppLogDefaults,
+    patchLegacyInfraLogDefaults,
+    patchLegacyAuditLogDefaults,
+    patchLegacyJsonLogging,
+    patchLegacyMultilineErrors,
+    patchLegacyCustomInputs,
+    patchLegacyCustomOutputs,
+    patchLegacyCustomPipelines,
   ],
   {
     inputs: {},
     outputs: {},
     pipelines: {},
-  }
-);
+  },
+) + com.makeMergeable(params.clusterLogForwarder);
 
 // ClusterLogForwarder:
 // Create definitive ClusterLogForwarder resource from specs.
@@ -176,8 +195,10 @@ local clusterLogForwarder = lib.ClusterLogForwarder(params.namespace, 'instance'
   },
 };
 
+local enableLogForwarder = std.length(params.clusterLogForwarder) > 0 || std.get(legacyConfig, 'enabled', false);
+
 // Define outputs below
-if params.clusterLogForwarding.enabled then
+if enableLogForwarder then
   {
     '31_cluster_logforwarding': clusterLogForwarder,
   }

tests/elasticsearch.yml

@@ -11,20 +11,12 @@ parameters:
       - type: https
         source: https://raw.githubusercontent.com/appuio/component-openshift4-monitoring/v2.9.0/lib/openshift4-monitoring-alert-patching.libsonnet
         output_path: vendor/lib/alert-patching.libsonnet
-      - type: https
-        source: https://raw.githubusercontent.com/projectsyn/component-patch-operator/v1.1.0/lib/patch-operator.libsonnet
-        output_path: vendor/lib/patch-operator.libsonnet
     compile:
       - input_type: jsonnet
         input_paths:
           - tests/console-patch.jsonnet
         output_path: console-patching/
 
-  patch_operator:
-    namespace: syn-patch-operator
-    patch_serviceaccount:
-      name: syn-patch-operator
-
   openshift4_operators:
     defaultInstallPlanApproval: Automatic
     defaultSource: openshift-operators-redhat

tests/forwardingonly.yml

@@ -28,6 +28,3 @@ parameters:
         enabled: false
       elasticsearch:
         enabled: false
-
-    clusterLogForwarding:
-      enabled: true

tests/golden/legacy/openshift4-logging/console-patching/openshift4_console_params.yaml

@@ -0,0 +1,3 @@
+config:
+  plugins:
+    - logging-view-plugin

tests/golden/legacy/openshift4-logging/openshift4-logging/00_namespace.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  annotations:
+    openshift.io/node-selector: ''
+  labels:
+    name: openshift-logging
+    openshift.io/cluster-monitoring: 'true'
+  name: openshift-logging

tests/golden/legacy/openshift4-logging/openshift4-logging/10_operator_group.yaml

@@ -0,0 +1,11 @@
+apiVersion: operators.coreos.com/v1
+kind: OperatorGroup
+metadata:
+  annotations: {}
+  labels:
+    name: cluster-logging
+  name: cluster-logging
+  namespace: openshift-logging
+spec:
+  targetNamespaces:
+    - openshift-logging

tests/golden/legacy/openshift4-logging/openshift4-logging/20_subscriptions.yaml

@@ -0,0 +1,43 @@
+apiVersion: operators.coreos.com/v1alpha1
+kind: Subscription
+metadata:
+  annotations: {}
+  labels:
+    name: cluster-logging
+  name: cluster-logging
+  namespace: openshift-logging
+spec:
+  channel: stable-5.9
+  config:
+    resources:
+      limits:
+        memory: 256Mi
+      requests:
+        cpu: 10m
+        memory: 128Mi
+  installPlanApproval: Automatic
+  name: cluster-logging
+  source: redhat-operators
+  sourceNamespace: openshift-operators-redhat
+---
+apiVersion: operators.coreos.com/v1alpha1
+kind: Subscription
+metadata:
+  annotations: {}
+  labels:
+    name: loki-operator
+  name: loki-operator
+  namespace: openshift-operators-redhat
+spec:
+  channel: stable-5.9
+  config:
+    resources:
+      limits:
+        memory: 512Mi
+      requests:
+        cpu: 50m
+        memory: 381Mi
+  installPlanApproval: Automatic
+  name: loki-operator
+  source: openshift-operators-redhat
+  sourceNamespace: openshift-operators-redhat

tests/golden/legacy/openshift4-logging/openshift4-logging/30_cluster_logging.yaml

@@ -0,0 +1,17 @@
+apiVersion: logging.openshift.io/v1
+kind: ClusterLogging
+metadata:
+  annotations:
+    argocd.argoproj.io/sync-options: SkipDryRunOnMissingResource=true
+  labels:
+    name: instance
+  name: instance
+  namespace: openshift-logging
+spec:
+  collection:
+    type: vector
+  logStore:
+    lokistack:
+      name: loki
+    type: lokistack
+  managementState: Managed