ADR-0002: How can we introduce a more general extension concept for data processing modules?


Status:	ACCEPTED
Date:	2020-05-20
Author(s):	Jannik Hollenbach Jannik.Hollenbach@iteratec.com, Jorge Estigarribia Jorge.Estigarribia@iteratec.com, Robert Seedorff Robert.Seedorff@iteratec.com, Sven Strittmatter Sven.Strittmatter@iteratec.com

Context

Status Quo

One major challenge implementing the secureCodeBox is to provide a flexible and modular architecture, which enables the open source community to easily understand the concepts and especially to extend the secureCodeBox with individual features. Therefore we decided to separate the process stages of a single security scan (instance of scanType custom resource definition; further abbreviated with CRD) in three major phases:

┌──────────────────┐          ┌──────────────────┐          ┌──────────────────┐
│     scanning     ├─────────▶│     parsing      ├─────────▶│    persisting    │
│    (phase 1)     │          │    (phase 2)     │          │    (phase 3)     │
└──────────────────┘          └──────────────────┘          └──────────────────┘

By now the phase 3 "persisting" was implemented by so called PersistenceProviders (e.g., the persistence-elastic provider which is responsible for persisting all findings in a given elasticsearch database). The secureCodeBox Operator is aware of this 3 phases and is responsible for the state model and execution of each security scan.

Problem and Question

We identified different additional use cases with a more "data processing oriented" pattern than the implemented phase 3 "persisting" indicates. For example, we implemented a so called MetaDataProvider feature, which is responsible for enhancing each security finding with additional metadata. But the MetaDataProvider must be executed after the phase 2 "parsing" and before the phase 3 "persisting" because it depends on the parsed finding results (which will be enhanced) and the updated findings should be also persisted.

To find a proper solution, we split the topic into the following two questions:

Should we unify the concepts MetaDataProvider and PersistenceProvider?
How should the execution model look like for each concept?

Question 1: Should We Unify the Concepts MetaDataProvider and PersistenceProvider?

Solution Approach 1: Unify

Both "modules" are "processing" the security findings, which were generated in the phase 2 "parsing", but there is one major difference between them:

a PersistenceProvider is processing the findings read only, and
a MetaDataProvider is processing the findings read and write.

There is a similar concept in Kubernetes called AdmissionController, but with the exception that the will be executed before a resource is created.

There are two variants of AdmissionControllers:

ValidatingWebhookConfiguration: read only, executed last; and
MutatingWebhookConfiguration: read and write, executed first.

We could do a similar thing and introduce CRD which allows to execute "custom code" (depends on the second question) after a scan has completed (meaning both phases "scan" and "parsing" were done). Some name ideas:

ScanHooks
ScanCompletionHooks
FindingProcessors

These could be implemented with a type attribute, which declares if they are read only or read and write.

The secureCodeBox operator would process all these CRDs in the namespace of the scan and execute the read and write ones first in serial only one at a time to avoid write conflicts and then the read only ones in parallel.

apiVersion: execution.securecodebox.io/v1
kind: ScanCompletionHook
metadata:
  name: my-metadata
spec:
  type: ReadAndWrite
  # If implemented like the current persistence provider
  image: my-metadata:v2.0.0

The Execution Flow would then look something like this:

                                                                                                           ┌ ReadOnly─Hooks─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─
                                              ┌ ReadAndWriteHooks ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─        ┌────────────────────────────────┐ │
                                                ┌───────────────────────┐                            │  ┌──┼▶│  Elastic PersistenceProvider   │
┌──────────────────┐   ┌──────────────────┐   │ │ ReadAndWrite Hook #1  │  ┌───────────────────────┐    │    └────────────────────────────────┘ │
│       Scan       ├──▶│     Parsing      │────▶│  "MyMetaDataProvider" ├─▶│ ReadAndWrite Hook #2  │─┼──┤  │ ┌────────────────────────────────┐
└──────────────────┘   └──────────────────┘   │ └───────────────────────┘  └───────────────────────┘    └───▶│ DefectDojo PersistenceProvider │ │
                                               ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘     │ └────────────────────────────────┘
                                                                                                            ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘

Pros

Only one implementation.
Pretty generic to expand and test out new ideas without having to modify the secureCodeBox operator.

Cons

Possibly an "over-abstraction".
Need to refactor the persistence-elastic provider.
The "general implementation" will be harder than the individual ones.

Solution Approach 2: Keep Split between Persistence Provider and MetaData Provider

Keep PersistenceProvider as they are and introduce new MetaDataProvider CRD which gets executed before the PersistenceProviders by the _secureCodeBox operator.

                                                                                                           ┌ Persistence Provider─ ─ ─ ─ ─ ─ ─ ─
                                              ┌ MetaData Provider ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─        ┌────────────────────────────────┐ │
                                                ┌───────────────────────┐                            │  ┌──┼▶│  Elastic PersistenceProvider   │
┌──────────────────┐   ┌──────────────────┐   │ │ ReadAndWrite Hook #1  │  ┌───────────────────────┐    │    └────────────────────────────────┘ │
│       Scan       ├──▶│     Parsing      │────▶│ "MyMetaDataProvider"  ├─▶│ ReadAndWrite Hook #2  │─┼──┤  │ ┌────────────────────────────────┐
└──────────────────┘   └──────────────────┘   │ └───────────────────────┘  └───────────────────────┘    └───▶│ DefectDojo PersistenceProvider │ │
                                               ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘     │ └────────────────────────────────┘
                                                                                                            ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘

Pros

Quicker to implement.
Might be worth it to have a separate concept for it.

Cons

Not sure if it worth to introduce a new CRD for everything, especially when it’s conceptually pretty close to to something already existing.

Question 2: How Should the Execution Model Look like for Each Concept?

Solution Approach 1: Like the Persistence Provider

Basically a docker container which process findings takes two arguments:

A pre-defined URL to download the findings from.
A pre-defined URL to upload the modified findings to.

Examples:

NodeJS: node my-metadata.js "https://storage.googleapi.com/..." "https://storage.googleapi.com/..."
Java: java my-metadata.jar "https://storage.googleapi.com/..." "https://storage.googleapi.com/..."
Golang: ./my-metadata "https://storage.googleapi.com/..." "https://storage.googleapi.com/..."

Pros

One liner with the current implementations.
Code overhead / wrapper code is pretty minimal.
Zero scale: no resource costs when nothing is running.

Cons

May results in too many Kubernetes jobs.
- Resource blocking on finished resources.
- ttlAfterFinished enabled.
Container runtime overhead (especially time).

Solution Approach 2: A WebHooks Like Concept

Analog to kubernetes webhooks: HTTP server receiving findings and returning results.

Pros

Milliseconds instead of seconds for processing.
No overhead for container Creation.
No additional kubernetes jobs needed.

Cons

Introduces new running services which needs to be maintained and have uptime.
Code overhead / boilerplate (Can be mitigated by an SDK).
Debugging of individual MetaDataProvider is harder than a single service which handles everything.
Introduces "new"cConcept.
Certificate management for webhook services (cert-manager required by default?).
Scaling for systems with lots of load could be a problem.
One service per namespace (multiple tenants) needed → results in many running active services which is resource consuming.

Decision

Regarding question 1 it seems that both solution approaches are resulting in the same execution model. We decided to implement solution approach 1 and unify both concepts into a more general concept with the name hook concept. Therefore we exchange the existing name PersistenceProvider for phase 3 in the execution model with a more general term processing:

┌──────────────────┐          ┌──────────────────┐          ┌──────────────────┐
│    scanning      ├─────────▶│    parsing       ├─────────▶│    processing    │
│    (Phase 1)     │          │    (Phase 2)     │          │    (Phase 3)     │
└──────────────────┘          └──────────────────┘          └──────────────────┘

Regarding question 2 we decided to implement the solution approach 1 with a job-based approach (no active service component needed). Therefore the phase 3 processing will be split into two separate phases named ReadAndWriteHooks (3.1) and ReadOnlyHooks (3.2):

                                                                                                           ┌ 3.2 processing: ReadOnlyHooks ─ ─ ─
                                              ┌ 3.1 processing: ReadAndWriteHooks ─ ─ ─ ─ ─ ─ ─ ─ ─ ─        ┌────────────────────────────────┐ │
                                                ┌───────────────────────┐                            │  ┌──┼▶│  Elastic PersistenceProvider   │
┌──────────────────┐   ┌──────────────────┐   │ │ ReadAndWrite Hook #1  │  ┌───────────────────────┐    │    └────────────────────────────────┘ │
│    scanning      ├──▶│     parsing      │────▶│  "MyMetaDataProvider" ├─▶│ ReadAndWrite Hook #2  │─┼──┤  │ ┌────────────────────────────────┐
└──────────────────┘   └──────────────────┘   │ └───────────────────────┘  └───────────────────────┘    └───▶│ DefectDojo PersistenceProvider │ │
                                               ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘     │ └────────────────────────────────┘
                                                                                                            ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘

Consequences

With the new hook concept we open the phase 3 processing to a more intuitive and flexible architecture. It is easier to understand because WebHooks are already a well known concept. It is possible to keep the existing implementation of the PersistenceProvider and integrate them with a lot of other possible processing components in a more general fashion. In the end, this step will result in a lot of additional feature possibilities, which go far beyond the existing ones proposed here. Therefore we only need to implement this concept once in the secureCodeBox operator and new ideas for extending the DataProcessing will not enforce conceptual or architectural changes.

Ideas for additional processing hooks:

Notifier hooks (ReadOnlyHook) e.g., for chat (slack, teams etc.), metric, alerting systems
MetaData enrichment hooks (ReadAndWriteHook)
FilterData hooks (ReadAndWriteHook) (e.g., false/positive handling)
SystemIntegration hooks (ReadOnlyHook) e.g., for ticketing systems like Jira
CascadingScans hooks (ReadOnlyHook) e.g., for starting new security scans based on findings

ADR-0002: How can we introduce a more general extension concept for data processing modules?

Context​

Status Quo​

Problem and Question​

Question 1: Should We Unify the Concepts MetaDataProvider and PersistenceProvider?​

Solution Approach 1: Unify​

Pros​

Cons​

Solution Approach 2: Keep Split between Persistence Provider and MetaData Provider​

Pros​

Cons​

Question 2: How Should the Execution Model Look like for Each Concept?​

Solution Approach 1: Like the Persistence Provider​

Pros​

Cons​

Solution Approach 2: A WebHooks Like Concept​

Pros​

Cons​

Decision​

Consequences​

Context

Status Quo

Problem and Question

Question 1: Should We Unify the Concepts MetaDataProvider and PersistenceProvider?

Solution Approach 1: Unify

Pros

Cons

Solution Approach 2: Keep Split between Persistence Provider and MetaData Provider

Pros

Cons

Question 2: How Should the Execution Model Look like for Each Concept?

Solution Approach 1: Like the Persistence Provider

Pros

Cons

Solution Approach 2: A WebHooks Like Concept

Pros

Cons

Decision

Consequences