-
Notifications
You must be signed in to change notification settings - Fork 845
Anti Patterns
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.
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.
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.
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 aput
step. This is usually done for optimization purposes, because theget
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 aput
that needs the resource, not realizing theget
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 theget
?".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.
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.
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.
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.
This adds tech debt that we'll just want to remove, and we already have the long-term fix in sight.
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.