Improve the error message arising from missing required configs for resource providers #1097
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swgillespie
added
kind/bug
|
|
lindydonna
approved these changes
Mar 30, 2018
pgavlin
approved these changes
Mar 31, 2018
| // variables, tfbridge will respond with a list of missing variables and their descriptions. | ||
| // If that is what occurred, we'll use that information here to construct a nice error message. | ||
| func createConfigureError(err *rpcerror.Error) error { | ||
| var aggregateErr error |
There was a problem hiding this comment.
Nit: how about renaming the parameter rpcerr and doing something like the following:
var err error = rpcerr
for _, detail := range rpcerr.Details() {
...
err = multierror.Append(err, singleError)
}
return err
sdk/proto/provider.proto
Outdated
| // ConfigureErrorMissingKeys is sent as a Detail on an error returned from `ResourceProvider.Configure`. | ||
| message ConfigureErrorMissingKeys { | ||
| message MissingKey { | ||
| string name = 1; // the Pulumi name (not the TF name!) of the missing config key. |
sdk/proto/provider.proto
Outdated
| message ConfigureErrorMissingKeys { | ||
| message MissingKey { | ||
| string name = 1; // the Pulumi name (not the TF name!) of the missing config key. | ||
| string description = 2; // a description of the missing config key, from the TF schema. |
There was a problem hiding this comment.
Do not mention TF here: not all providers will be TF-based
|
Likewise, do not mention TF/tfbridge/etc. in the commit message. Resource providers != terraform in the general case. |
resource providers If the resource provider that we are speaking to is new enough, it will send across a list of keys and their descriptions alongside an error indicating that the provider we are configuring is missing required config. This commit packages up the list of missing keys into an error that can be presented nicely to the user.
swgillespie
force-pushed
the
swgillespie/config-errors-2
branch
from
20b355e
to
53c7bac
Compare
swgillespie
force-pushed
the
swgillespie/config-errors-2
branch
from
53c7bac
to
3d693bd
Compare
|
thanks for the reviews! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the tfbridge that we are speaking to is new enough, it will send
across a list of keys and their descriptions alongside an error
indicating that the provider we are configuring is missing required
config. This commit packages up the list of missing keys into an error
that can be presented nicely to the user.
Take 2! This is the first of two PRs that implement this new error message functionality; a companion PR will be opened in
pulumi-terraformthat implements the server portion of these changes. Because of the way our Go dependency graph is shaped, this PR will need to be merged before thepulumi-terraformone.The idea here is that, before
tfbridgeasks Terraform to validate a set of configs, it will first check the list of config keys we give it against the list of configs known to us (through the TF Schema) to be required. Iftfbridgesees any keys that it does not recognize, it'll send an error response to theConfigureRPC call and attach aConfigureErrorMissingKeysdetail to the error, which we as the plugin host can interpret and provide a specific error message for each missing key.With all pieces in place (including updates to
pulumi-awsandpulumi-terraform, which will come from follow-up PRs), this is the new output:For the changelog:
This change improves the error message that appears whenever a required configuration key is not provided, such as
aws:region. The error message will now print out the names of all missing configuration keys as well as a description of the key's function.