Anti Patterns

Jacob Martin edited this page Apr 23, 2018 · 3 revisions

CI systems have a lot of feature requests. Their user base is basically anyone writing software. All kinds of software. As a result, we receive many specific requests, often for things we explicitly do not want to do, for risk of becoming another hulking mass of complicated software with no principles.

The following anti-patterns correspond to GitHub labels which we'll use to mark these issues or pull-requests, to better express the Concourse ideology and hopefully provide more context as to why something was turned down.

contributor-burden

This issue or pull request demonstrates pattern that would require effort from every resource author to keep up. It also means each resource author may implement this differently, and possibly with bugs or security risks that necessitate constant maintenance or backwards-incompatible changes.

A few common examples:

  • Credential acquisition: supporting AWS IAM roles or other ways for a resource to acquire credentials places a burden on every resource author to implement the same thing. This also goes for GCP or other IaaSes that implement similar features. Implementation and upkeep quickly turns this into an M*N problem.

  • Certificate management: this is an unsolved problem today - see https://github.com/concourse/concourse/issues/1027 where we're thinking up a proposal.

  • Dynamic configuration: resources should only have to implement support for static, self-contained request payloads. Some workflows introduce a need for more dynamic configuration, but this currently requires the resource author to implement it explicitly. There will also be varying use cases that call for varying params to be dynamic, and this may grow over time for an individual resource, leading to multiple ways for everything to be configured.

    A generic proposal for solving this existing problem exists in https://github.com/concourse/concourse/issues/684.

duplicate-effort

This is a problem outside the scope of our team. We should not be spending time on this, as the problem should be solved by the source of truth (i.e. maintainers of the dependency).

A common example of this is documentation. We should not be documenting the multitude of different ways to deploy and configure BOSH or PostgreSQL or Docker. We are not the experts, and our documentation would likely become out of date or suggest the wrong thing.

Another example would be working around deficiencies in a dependency or deployment scenario.

knobs

This introduces a knob or switch that will require constant or, worse, occasional attention from people using or deploying Concourse. Usually when someone asks for something like this, there's a deeper need that's not being met, which should take more thinking, not just a way out (similar to quick-fix).

A few examples:

  • Being able to disable the automatic get after a put step. This is usually done for optimization purposes, because the get can be expensive. There are many problems with turning this knob.

    The first problem with turning this knob is that you may have to turn it again later. An engineer may add a task or a put that needs the resource, not realizing the get was disabled, leading to confusion and failed builds until they figure it out.

    The second problem is that it introduces mental overhead to pipeline maintenance: now every put will come with the question of "should I disable the get?".

    This particular request is pretty common, and there are tactical approaches we can take to resolve the specific reasons the user may want it disabled. For example, we could know not to fetch the resource if no later steps require it, or only fetching it once we know a step requires it. There are difficulties here technically and also UX-wise, which make it a lower priority, but we don't want to just say "no" to this forever, as there is a clear need.

  • Any configuration that changes the semantics of Concourse globally across all teams. This makes pipelines non-self-contained, which goes against the principle of portable and reproducible pipelines.

worker-state

This leaks through the abstractions Concourse provides around self-contained tasks and resources. Workers generally should not be hand-configured expecting certain workloads to run on them. Credential access should not be tied to workers, as they may be running untrusted workloads (e.g. pull requests).

Tasks should be self-contained, by using an image that provides all of their runtime dependencies, and explicitly taking anything that may change their result as inputs.

magic

This introduces functionality that, while it may be cool, may introduce a delay in understanding of Concourse fundamentals. Using Concourse effectively requires a deep understanding of what few primitives it provides, and why we're able to have so few primitives to express so many things.

multi-source-of-truth

This repeats information which can already be obtained from the source of truth.

An example of this would be taking metadata from the git resource (e.g. the ref or author of the commit) and exposing it to tasks via environment variables. This breaks the resource abstraction introduces a coupling to git that may require maintenance over time. It may also be complicated to implement in a way that keeps tasks consistent between fly execute and pipeline semantics.

For this reason, we do not expose any resource-specific metadata, instead distilling resources, task inputs, and task outputs to just "files on disk", which can be inspected with standard tooling. Tasks can ensure those tools (e.g. the git command) are available by using an image that has them.

quick-fix

This adds tech debt that we'll just want to remove, and we already have the long-term fix in sight.

kitchen-sink-config

By allowing the wildcard "set any env vars or config flags you want", we can no longer know what features people are relying on, and the code becomes dangerous to refactor.

This first became apparent in the Docker Image resource. A PR added a server_args param, allowing any flags to be passed to the dockerd server. This was then depended on by users for configuring things like --insecure-registries. One day though, we decided to refactor the check command to no longer run dockerd, and instead use native Go Docker APIs. This made server_args no longer relevant for check, and thus made it impossible to support insecure registries.

If a feature is to be supported, it should have a high-level configuration. Resources should have high-level settings in params for everything, and understand every input. Even if it means stringing together a bunch of pass-through configs.

You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.
Press h to open a hovercard with more details.