Skip to content

Commit 10cdde3

Browse files
gdavisongdavison
authored
Merge pull request #47750 from hashicorp/td-no-validate-model-at-runtime
Provider: Removes run-time validation for Framework-based resource types
2 parents 53398e5 + d57c68e commit 10cdde3

5 files changed

Lines changed: 223 additions & 235 deletions

File tree

‎GNUmakefile‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,13 @@ sanity: prereq-go ## Run sanity check (failures allowed)
603603
exit 1; \
604604
fi
605605

606+
schema-validate: ## Validate schemas
607+
@echo "make: Validating schemas"
608+
@$(GO_VER) test -vet=off -buildvcs=false \
609+
./internal/provider/sdkv2 -run=TestProviderInit
610+
@$(GO_VER) test -vet=off -buildvcs=false \
611+
./internal/provider/framework -run=TestProviderInit
612+
606613
semgrep: semgrep-code-quality semgrep-naming semgrep-naming-cae semgrep-service-naming ## [CI] Run all CI Semgrep checks
607614

608615
semgrep-all: semgrep-test semgrep-validate ## Run semgrep on all files
@@ -912,7 +919,7 @@ test-shard: prereq-go ## Run unit tests for a specific shard (CI only: SHARD=0 T
912919
test-naming: ## Check test naming conventions
913920
@.ci/scripts/check-test-naming.sh
914921

915-
testacc: prereq-go fmt-check ## Run acceptance tests
922+
testacc: prereq-go fmt-check schema-validate ## Run acceptance tests
916923
@branch=$$(git rev-parse --abbrev-ref HEAD); \
917924
printf "make: Running acceptance tests on branch: \033[1m%s\033[0m...\n" "🌿 $$branch 🌿"
918925
@if [ "$(TESTARGS)" = "-run=TestAccXXX" ]; then \
@@ -925,7 +932,7 @@ testacc: prereq-go fmt-check ## Run acceptance tests
925932
echo "See the contributing guide for more information: https://hashicorp.github.io/terraform-provider-aws/running-and-writing-acceptance-tests"; \
926933
exit 1; \
927934
fi
928-
TF_ACC=1 $(GO_VER) test ./$(PKG_NAME)/... -v -count $(TEST_COUNT) -parallel $(ACCTEST_PARALLELISM) $(RUNARGS) $(TESTARGS) -timeout $(ACCTEST_TIMEOUT) -vet=off
935+
TF_ACC=1 $(GO_VER) test ./$(PKG_NAME)/... -v -count $(TEST_COUNT) -parallel $(ACCTEST_PARALLELISM) $(RUNARGS) $(TESTARGS) -timeout $(ACCTEST_TIMEOUT) -vet=off -buildvcs=false
929936

930937
testacc-lint: ## [CI] Acceptance Test Linting / terrafmt
931938
@echo "make: Acceptance Test Linting / terrafmt..."

‎docs/makefile-cheat-sheet.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ Variables are often defined before the `make` call on the same line, such as `MY
146146
| `provider-markdown-lint` | Provider Check / markdown-lint | ✔️ | | |
147147
| `sane`<sup>D</sup> | Run sane check | | | `ACCTEST_PARALLELISM`, `ACCTEST_TIMEOUT`, `GO_VER`, `TEST_COUNT` |
148148
| `sanity`<sup>D</sup> | Run sanity check (failures allowed) | | | `ACCTEST_PARALLELISM`, `ACCTEST_TIMEOUT`, `GO_VER`, `TEST_COUNT` |
149+
| `schema-validate` | Validate schemas | | | `GO_VER` |
149150
| `semgrep`<sup>M</sup> | Run all CI Semgrep checks | ✔️ | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |
150151
| `semgrep-all`<sup>D</sup> | Run semgrep on all files | | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |
151152
| `semgrep-code-quality`<sup>D</sup> | Semgrep Checks / Code Quality Scan | ✔️ | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |

‎internal/provider/framework/provider.go‎

Lines changed: 0 additions & 207 deletions
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,26 @@ package framework
55

66
import (
77
"context"
8-
"errors"
9-
"fmt"
108
"iter"
119
"log"
12-
"reflect"
1310
"slices"
14-
"sync"
15-
"unique"
1611

1712
"github.com/hashicorp/terraform-plugin-framework-timetypes/timetypes"
1813
"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
1914
"github.com/hashicorp/terraform-plugin-framework/action"
20-
aschema "github.com/hashicorp/terraform-plugin-framework/action/schema"
2115
"github.com/hashicorp/terraform-plugin-framework/datasource"
22-
datasourceschema "github.com/hashicorp/terraform-plugin-framework/datasource/schema"
2316
"github.com/hashicorp/terraform-plugin-framework/ephemeral"
24-
empemeralschema "github.com/hashicorp/terraform-plugin-framework/ephemeral/schema"
2517
"github.com/hashicorp/terraform-plugin-framework/function"
2618
"github.com/hashicorp/terraform-plugin-framework/list"
2719
"github.com/hashicorp/terraform-plugin-framework/provider"
2820
"github.com/hashicorp/terraform-plugin-framework/provider/metaschema"
2921
"github.com/hashicorp/terraform-plugin-framework/provider/schema"
3022
"github.com/hashicorp/terraform-plugin-framework/resource"
31-
resourceschema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
3223
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
3324
"github.com/hashicorp/terraform-plugin-framework/types"
3425
"github.com/hashicorp/terraform-provider-aws/internal/conns"
35-
"github.com/hashicorp/terraform-provider-aws/internal/framework"
3626
tffunction "github.com/hashicorp/terraform-provider-aws/internal/function"
3727
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
38-
inttypes "github.com/hashicorp/terraform-provider-aws/internal/types"
39-
tfunique "github.com/hashicorp/terraform-provider-aws/internal/unique"
40-
"github.com/hashicorp/terraform-provider-aws/names"
41-
)
42-
43-
var (
44-
resourceSchemasValidated sync.Once
4528
)
4629

4730
var (
@@ -77,16 +60,6 @@ func NewProvider(ctx context.Context, primary interface{ Meta() any }) (provider
7760
servicePackages: primary.Meta().(*conns.AWSClient).ServicePackages(ctx),
7861
}
7962

80-
// Because we try and share resource schemas as much as possible,
81-
// we need to ensure that we only validate the resource schemas once.
82-
var err error
83-
resourceSchemasValidated.Do(func() {
84-
err = provider.validateResourceSchemas(ctx)
85-
})
86-
if err != nil {
87-
return nil, err
88-
}
89-
9063
provider.initialize(ctx)
9164

9265
return provider, nil
@@ -484,183 +457,3 @@ func (p *frameworkProvider) initialize(ctx context.Context) {
484457
}
485458
}
486459
}
487-
488-
// validateResourceSchemas is called from `New` to validate Terraform Plugin Framework-style resource schemas.
489-
func (p *frameworkProvider) validateResourceSchemas(ctx context.Context) error {
490-
var errs []error
491-
492-
for sp := range p.servicePackages {
493-
for _, dataSourceSpec := range sp.FrameworkDataSources(ctx) {
494-
typeName := dataSourceSpec.TypeName
495-
inner, err := dataSourceSpec.Factory(ctx)
496-
497-
if err != nil {
498-
errs = append(errs, fmt.Errorf("creating data source type (%s): %w", typeName, err))
499-
continue
500-
}
501-
502-
schemaResponse := datasource.SchemaResponse{}
503-
inner.Schema(ctx, datasource.SchemaRequest{}, &schemaResponse)
504-
505-
if err := validateSchemaRegionForDataSource(dataSourceSpec.Region, schemaResponse.Schema); err != nil {
506-
errs = append(errs, fmt.Errorf("data source type %q: %w", typeName, err))
507-
continue
508-
}
509-
510-
if err := validateSchemaTagsForDataSource(dataSourceSpec.Tags, schemaResponse.Schema); err != nil {
511-
errs = append(errs, fmt.Errorf("data source type %q: %w", typeName, err))
512-
continue
513-
}
514-
}
515-
516-
if v, ok := sp.(conns.ServicePackageWithEphemeralResources); ok {
517-
for _, ephemeralResourceSpec := range v.EphemeralResources(ctx) {
518-
typeName := ephemeralResourceSpec.TypeName
519-
inner, err := ephemeralResourceSpec.Factory(ctx)
520-
521-
if err != nil {
522-
errs = append(errs, fmt.Errorf("creating ephemeral resource type (%s): %w", typeName, err))
523-
continue
524-
}
525-
526-
schemaResponse := ephemeral.SchemaResponse{}
527-
inner.Schema(ctx, ephemeral.SchemaRequest{}, &schemaResponse)
528-
529-
if err := validateSchemaRegionForEphemeralResource(ephemeralResourceSpec.Region, schemaResponse.Schema); err != nil {
530-
errs = append(errs, fmt.Errorf("ephemeral resource type %q: %w", typeName, err))
531-
continue
532-
}
533-
}
534-
}
535-
536-
if v, ok := sp.(conns.ServicePackageWithActions); ok {
537-
for _, actionSpec := range v.Actions(ctx) {
538-
typeName := actionSpec.TypeName
539-
inner, err := actionSpec.Factory(ctx)
540-
541-
if err != nil {
542-
errs = append(errs, fmt.Errorf("creating action type (%s): %w", typeName, err))
543-
continue
544-
}
545-
546-
schemaResponse := action.SchemaResponse{}
547-
inner.Schema(ctx, action.SchemaRequest{}, &schemaResponse)
548-
549-
if err := validateSchemaRegionForAction(actionSpec.Region, schemaResponse.Schema); err != nil {
550-
errs = append(errs, fmt.Errorf("action type %q: %w", typeName, err))
551-
continue
552-
}
553-
}
554-
}
555-
556-
for _, resourceSpec := range sp.FrameworkResources(ctx) {
557-
typeName := resourceSpec.TypeName
558-
inner, err := resourceSpec.Factory(ctx)
559-
560-
if err != nil {
561-
errs = append(errs, fmt.Errorf("creating resource type (%s): %w", typeName, err))
562-
continue
563-
}
564-
565-
schemaResponse := resource.SchemaResponse{}
566-
inner.Schema(ctx, resource.SchemaRequest{}, &schemaResponse)
567-
568-
if err := validateSchemaRegionForResource(resourceSpec.Region, schemaResponse.Schema); err != nil {
569-
errs = append(errs, fmt.Errorf("resource type %q: %w", typeName, err))
570-
continue
571-
}
572-
573-
if err := validateSchemaTagsForResource(resourceSpec.Tags, schemaResponse.Schema); err != nil {
574-
errs = append(errs, fmt.Errorf("resource type %q: %w", typeName, err))
575-
continue
576-
}
577-
578-
if resourceSpec.Import.WrappedImport {
579-
if resourceSpec.Import.SetIDAttr {
580-
if _, ok := resourceSpec.Import.ImportID.(inttypes.FrameworkImportIDCreator); !ok {
581-
errs = append(errs, fmt.Errorf("resource type %q: importer sets `%s` attribute, but creator isn't configured", resourceSpec.TypeName, names.AttrID))
582-
continue
583-
}
584-
}
585-
586-
if _, ok := inner.(framework.ImportByIdentityer); !ok {
587-
errs = append(errs, fmt.Errorf("resource type %q: cannot configure importer, does not implement %q", resourceSpec.TypeName, reflect.TypeFor[framework.ImportByIdentityer]()))
588-
continue
589-
}
590-
}
591-
}
592-
}
593-
594-
return errors.Join(errs...)
595-
}
596-
597-
func validateSchemaRegionForDataSource(regionSpec unique.Handle[inttypes.ServicePackageResourceRegion], schema datasourceschema.Schema) error {
598-
if !tfunique.IsHandleNil(regionSpec) && regionSpec.Value().IsOverrideEnabled {
599-
if _, ok := schema.Attributes[names.AttrRegion]; ok {
600-
return fmt.Errorf("configured for enhanced regions but defines `%s` attribute in schema", names.AttrRegion)
601-
}
602-
}
603-
return nil
604-
}
605-
606-
func validateSchemaRegionForEphemeralResource(regionSpec unique.Handle[inttypes.ServicePackageResourceRegion], schema empemeralschema.Schema) error {
607-
if !tfunique.IsHandleNil(regionSpec) && regionSpec.Value().IsOverrideEnabled {
608-
if _, ok := schema.Attributes[names.AttrRegion]; ok {
609-
return fmt.Errorf("configured for enhanced regions but defines `%s` attribute in schema", names.AttrRegion)
610-
}
611-
}
612-
return nil
613-
}
614-
615-
func validateSchemaRegionForAction(regionSpec unique.Handle[inttypes.ServicePackageResourceRegion], schemaIface any) error {
616-
if !tfunique.IsHandleNil(regionSpec) && regionSpec.Value().IsOverrideEnabled {
617-
if schema, ok := schemaIface.(aschema.Schema); ok {
618-
if _, ok := schema.Attributes[names.AttrRegion]; ok {
619-
return fmt.Errorf("configured for enhanced regions but defines `%s` attribute in schema", names.AttrRegion)
620-
}
621-
}
622-
}
623-
return nil
624-
}
625-
626-
func validateSchemaRegionForResource(regionSpec unique.Handle[inttypes.ServicePackageResourceRegion], schema resourceschema.Schema) error {
627-
if !tfunique.IsHandleNil(regionSpec) && regionSpec.Value().IsOverrideEnabled {
628-
if _, ok := schema.Attributes[names.AttrRegion]; ok {
629-
return fmt.Errorf("configured for enhanced regions but defines `%s` attribute in schema", names.AttrRegion)
630-
}
631-
}
632-
return nil
633-
}
634-
635-
func validateSchemaTagsForDataSource(tagsSpec unique.Handle[inttypes.ServicePackageResourceTags], schema datasourceschema.Schema) error {
636-
if !tfunique.IsHandleNil(tagsSpec) {
637-
if v, ok := schema.Attributes[names.AttrTags]; ok {
638-
if !v.IsComputed() {
639-
return fmt.Errorf("`%s` attribute must be Computed", names.AttrTags)
640-
}
641-
} else {
642-
return fmt.Errorf("configured for tags but no `%s` attribute defined in schema", names.AttrTags)
643-
}
644-
}
645-
return nil
646-
}
647-
648-
func validateSchemaTagsForResource(tagsSpec unique.Handle[inttypes.ServicePackageResourceTags], schema resourceschema.Schema) error {
649-
if !tfunique.IsHandleNil(tagsSpec) {
650-
if v, ok := schema.Attributes[names.AttrTags]; ok {
651-
if v.IsComputed() {
652-
return fmt.Errorf("`%s` attribute cannot be Computed", names.AttrTags)
653-
}
654-
} else {
655-
return fmt.Errorf("configured for tags but no `%s` attribute defined in schema", names.AttrTags)
656-
}
657-
if v, ok := schema.Attributes[names.AttrTagsAll]; ok {
658-
if !v.IsComputed() {
659-
return fmt.Errorf("`%s` attribute must be Computed", names.AttrTagsAll)
660-
}
661-
} else {
662-
return fmt.Errorf("configured for tags but no `%s` attribute defined in schema", names.AttrTagsAll)
663-
}
664-
}
665-
return nil
666-
}

0 commit comments

Comments
 (0)