Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider Raising Error Diagnostic If Any Diagnostic Is Missing Severity #285

Open
bflad opened this issue Apr 21, 2023 · 0 comments
Open
Labels
enhancement New feature or request

Comments

@bflad
Copy link
Member

bflad commented Apr 21, 2023

terraform-plugin-go version

v0.15.0

Use cases

When creating tfprotov5.Diagnostic or tfprotov6.Diagnostic, it is possible to omit the diagnostic Severity field. While the severity has a fallback "invalid" level and Terraform will generally stop execution, it can cause issues with terraform-exec not reporting the diagnostic in its error return for commands. Provider acceptance testing via terraform-plugin-testing using terraform-exec means tests can have undefined behavior to real usage, such as potentially passing tests unexpectedly and not alerting provider developers to a real issue.

Attempted solutions

If there was something to be done here, it would likely be creating static analysis tooling to catch the issue.

Proposal

Rather than a more drastic change in this Go module of making it not possible to create diagnostics without a severity, we can consider augmenting the toproto implementations to raise their own error diagnostic if they detect response diagnostics missing a severity.

Potentially something like:

func Diagnostics(in []*tfprotov5.Diagnostic) ([]*tfplugin5.Diagnostic, error) {
	diagnostics := make([]*tfplugin5.Diagnostic, 0, len(in))
	for _, diag := range in {
		if diag == nil {
			diagnostics = append(diagnostics, nil)
			continue
		}
		d, err := Diagnostic(diag)
		if err != nil {
			return diagnostics, err
		}
		diagnostics = append(diagnostics, d)
		if diag.Severity == tfprotov5.DiagnosticSeverityInvalid { // new logic
			diagnostics = append(
				diagnostics,
				tfplugin5.Diagnostic{
					Severity: tfplugin5.Diagnostic_ERROR,
					Summary: "Diagnostic Missing Severity",
					Detail: "A response diagnostic was missing a severity level. "+
						"This can cause consumers of Terraform command outputs to miss the diagnostic, which can cause unexpected behavior. "+
						"Please report this issue to the provider developers.\n\n"+
						"Summary for Diagnostic Missing Severity: "+diag.Summary, 
				},
			)
		}
	}
	return diagnostics, nil
}

References

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant