Feature: Pipeline resource validation

[eczy-tempus]

Pipeline Resource Validation

This PR adds validation to the concourse_pipeline resource using the same validation function that is used in fly validate-pipeline (i.e. this is equivalent to fly validate-pipeline in non-strict mode). This allows pipeline errors to be caught when running terraform plan rather waiting for failures when running terraform apply.

Implementation

The validation has been implemented as a schema behavior on the pipeline_config attribute. We considered adding this as an optional feature, but because this is implemented as a schema behavior it is not possible to access other schema values within the validation block. It would be possible to add this as an optional feature, but it would require experimental terraform sdk features and more complex changes to the existing provider. Additionally, any concourse pipeline in active use must necessarily be a valid pipeline (therefore passing this validation), and so enabling validation by default as it is in this PR should not break any standard concourse workflows. I am open to opposing opinions on this, and I can re-implement this as an optional feature if necessary.

Note: this required an upgrade to the concourse dependency, which led to several insertions of atc.PipelineRef where pipeline names were previously passed as strings.

[risicle]

Hold on, isn't whether a pipeline is valid or not concourse-version-dependent? Validating it locally means we have to choose a concourse version to validate for - which do we choose?

At the moment we just about get away with using a mismatched client library against any (and we can't guarantee it works with any) version of concourse it is pointed at because in practise the protocol for setting/getting pipelines/teams doesn't change much. But new pipeline features appear all the time.

[eczy-tempus]

Yes, validating a pipeline is technically dependent on the version of concourse, though that particular code path is relatively infrequently updated (last significant validation change was a year ago).

In my opinion, this provider should be built against the latest stable concourse version. Users generally should be running recent versions of concourse, and those who aren’t can use older builds of this provider that support their target version. The concourse API shouldn’t break unless a major version is released anyway, which is infrequent. This would also prevent future issues which may arise due to client-server version mismatch.

Based on what you’ve said, it sounds like taking the above approach wouldn’t considerably impact the current level of compatibility, and it actually might improve it with more explicit version matching.

[jfharden]

@eczy-tempus If we have to match provider versions to concourse versions it will create a lot of TOIL having to update the provider for every concourse release, and should we need to update the provider for any feature (such as the ability to import pipelines as was just merged yesterday) that feature could only be used by people on the latest version of concourse unless we backport it to all the previous versions.

While I agree people should be using the most recent version of concourse experience tells us this isn't the case and I don't think we want to deny new provider improvements and features to people because they are pinned to older concourse versions.

I'm of the opinion that validating the actual json being sent to concourse isn't the job of the provider, personally I would include it as an extra step in whatever tooling is being used for test and deployment (git pre-commit hook, github actions build, concourse pipeline job etc) which means it can match the fly version to the concourse version in use.

[eczy-tempus]

@jfharden

I don’t personally view keeping dependencies up to date as an unacceptable level of toil since it is rather unavoidable, and it is often more painful in the future to put it off. My concern is that currently the provider loosely supports many concourse versions by using an out-of-date dependency rather than explicitly supporting many concourse versions with explicit dependency management. The current Concourse dependency in master branch is pinned to a git ref from August 2020, and so this provider is essentially hoping that no breaking changes have occurred in go-concourse since then. This is by no means guaranteed (I already had to modify some concourse.Team function calls in this PR due to updating this dependency).

If you are concerned with people on older versions of Concourse having access to newer provider features, would it not be possible to maintain builds for major Concourse versions e.g. have a version for Concourse 6.x.x, Concourse 7.x.x, and so on for newer major versions? This way users of older versions are still supported and users of newer versions do not miss out on newer features and risk compatibility issues.

I’m of the opinion that validating the actual json being sent to concourse isn’t the job of the provider

Personally, I think that validation is generally very much in the scope of what Terraform providers should provide. In many cases a provider may not be able to achieve comprehensive validation (state external to Terraform, etc.), but at the very least a provider should be able to detect invalid inputs at plan time instead of waiting for a failed apply.

I would include it as an extra step in whatever tooling is being used for test and deployment (git pre-commit hook, github actions build, concourse pipeline job etc) which means it can match the fly version to the concourse version in use.

This might be an option for teams which pass the yaml to the provider pre-rendered, but for teams that use Terraform’s template rendering to parameterize their yaml files (like ours) the provider is the only practical place where this validation can occur.

If this is too much additional work, some members of my team and I would be happy to assist as maintainers. We depend upon this provider heavily for our Concourse tooling, so allowing us to manage some of this additional toil would be mutually beneficial.

[eczy-tempus]

I am closing this PR as it is now very out of date and apparently unlikely to be addressed. I maintain that validation is very much within the scope of what a provider should support. I will gladly reopen and update this PR if the maintainers indicate that they would be interested in such work.