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

command: Fix backend config override validation #25960

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

alisdair
Copy link
Contributor

@alisdair alisdair commented Aug 21, 2020

When loading a backend config override file, init was doing two things wrong:

  • First, if the file failed to parse, we accidentally didn't return, which caused a panic due to the parsed body being nil;
  • Secondly, we were overzealous with the validation of the file, allowing only attributes. While most backend configs are attributes only, the enhanced remote backend body also contains a workspaces block, which we need to support here.

This commit fixes the first bug with an early return and adds test cases for missing file and intentionally-blank filename (to clear the config).

We also add a schema validation for the backend block, based on the backend schema itself. This requires constructing an HCL body schema so that we can call Content and check for diagnostic errors.

The result is more useful errors when an invalid backend config override file is used, while also supporting the enhanced remote backend config fully.

Does not include tests specific to the remote backend, because the mocking involved to allow the backend to fully initialize is too involved to be worth it. Here's what that looks like: 159cdd3

Fixes #25793

When loading a backend config override file, init was doing two things
wrong:

- First, if the file failed to parse, we accidentally didn't return,
  which caused a panic due to the parsed body being nil;
- Secondly, we were overzealous with the validation of the file,
  allowing only attributes. While most backend configs are attributes
  only, the enhanced remote backend body also contains a `workspaces`
  block, which we need to support here.

This commit fixes the first bug with an early return and adds test cases
for missing file and intentionally-blank filename (to clear the config).

We also add a schema validation for the backend block, based on the
backend schema itself. This requires constructing an HCL body schema so
that we can call `Content` and check for diagnostic errors.

The result is more useful errors when an invalid backend config override
file is used, while also supporting the enhanced remote backend config
fully.

Does not include tests specific to the remote backend, because the
mocking involved to allow the backend to fully initialize is too
involved to be worth it.
@alisdair alisdair requested a review from a team August 21, 2020 20:22
@alisdair alisdair self-assigned this Aug 21, 2020
@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #25960 into master will decrease coverage by 0.00%.
The diff coverage is 57.89%.

Impacted Files Coverage Δ
command/init.go 67.14% <57.89%> (-0.67%) ⬇️
dag/marshal.go 54.44% <0.00%> (+1.11%) ⬆️

Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 🎉 ! We can close #25804 once this gets merged

@ghost
Copy link

ghost commented Oct 11, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

init command should support workspaces block in backend config file
2 participants