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

(TF-18290) Early validation for Terraform Stack files #1776

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Jul 24, 2024

This change enables early validation for Terraform Stacks in tfstack. hcl and tfdeploy.hcl files.

Available validations:

  • Missing required attributes
  • Deprecated attributes
  • Deprecated blocks
  • Unexpected attributes
  • Unexpected blocks
  • Maximum number of blocks
  • Minimum number of blocks
  • Block labels length

This adds a new job to validate the schema of the these files. The job is enqueued when the EnableEnhancedValidation option is set to true.

@jpogran jpogran self-assigned this Jul 24, 2024
@jpogran jpogran changed the base branch from main to pre-release July 24, 2024 16:15
@jpogran jpogran changed the title stack early validation (TF-18290) Early validation for Terraform Stack files Jul 24, 2024
@jpogran jpogran marked this pull request as ready for review July 24, 2024 16:25
@jpogran jpogran requested a review from a team as a code owner July 24, 2024 16:25
@jpogran jpogran force-pushed the stack_early_validation branch 2 times, most recently from 059eef6 to 7d4b95d Compare July 24, 2024 16:33
Copy link
Member

@ansgarm ansgarm left a comment

Choose a reason for hiding this comment

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

Just two small questions – but it's looking good for all that I understand!

internal/features/stacks/events.go Outdated Show resolved Hide resolved
@jpogran jpogran marked this pull request as draft July 25, 2024 14:01
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Nice draft! I left some thoughts

internal/features/stacks/events.go Outdated Show resolved Hide resolved
internal/features/stacks/events.go Outdated Show resolved Hide resolved
internal/features/stacks/events.go Outdated Show resolved Hide resolved
internal/features/stacks/jobs/validation.go Outdated Show resolved Hide resolved
internal/features/stacks/jobs/validation.go Outdated Show resolved Hide resolved
@jpogran jpogran marked this pull request as ready for review July 31, 2024 20:03
Copy link
Member

@ansgarm ansgarm left a comment

Choose a reason for hiding this comment

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

This looks really good! Just a few small cleanup things mostly.

I'm fine with having just one decoder for didChange events 👍 We might need to change that once we support references between stack and deployment files (for inputs) but that's for later!

internal/features/stacks/decoder/path_reader.go Outdated Show resolved Hide resolved
internal/features/stacks/decoder/validations/valid_name.go Outdated Show resolved Hide resolved
internal/features/stacks/events.go Outdated Show resolved Hide resolved
internal/features/stacks/events.go Outdated Show resolved Hide resolved
internal/features/stacks/jobs/validation.go Show resolved Hide resolved
This change enables early validation for Terraform Stacks in tfstack. hcl and tfdeploy.hcl files.

Available validations:

- Missing required attributes
- Deprecated attributes
- Deprecated blocks
- Unexpected attributes
- Unexpected blocks
- Maximum number of blocks
- Minimum number of blocks
- Block labels length

This adds a new job to validate the schema of the these files. The job is enqueued when the `EnableEnhancedValidation` option is set to true.
TODO: Revist checking Stack concepts in the static validators

Other validators are more generic and can be applied to any block/attribute/etc, but this one is specific to the stack block types. I'm sure this could be done in a more generic way, but I'm not sure if it's worth it at the moment.

The names of these blocks are used as identifiers in the stack and are used in the UI, so they have to be valid Terraform identifiers or else the stack will not be able to be created. This is a very basic check to ensure that the names are valid until we can do more advanced checks or use the cloud api.

I tried adding this to the earlydecoder, but we do not seem to report the diagnostics there at all currently, so nothing was being reported. I'm not sure if that's a bug or intended.
@jpogran jpogran merged commit 5e25bd3 into pre-release Aug 5, 2024
6 checks passed
@jpogran jpogran deleted the stack_early_validation branch August 5, 2024 15:28
Copy link

github-actions bot commented Sep 5, 2024

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2024
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.

3 participants