Skip to content

Commit

Permalink
chore: Improve config builders (#3207)
Browse files Browse the repository at this point in the history
Improve how we build config models:
- explore HCL v1 lib
- generate provider config builder
- generate datasource config builder
- generate config using hcl v1 lib
- support objects, lists, and sets
- use provider config in provider setup tests
- use datasource generated config in one test for databases datasource
- use support for objects and lists in one view resource acceptance test
- extract nullVariable to own file
- cleanup in bettertestspoc/config directory
- add all datasources to provider/datasources
- ~deprecate old config builder and config variables from config
methods~ - undeprecated for now because of linter (and I could not
quickly find how to disable it; will do in a follow-up)
- multiline values (using <<EOT...EOT) are handled far from ideally but
work for now

References:
- https://developer.hashicorp.com/terraform/language/syntax/json
-
https://github.com/hashicorp/hcl/blob/56a9aee5207dbaed7f061cd926b96fc159d26ea0/json/spec.md

TODOs (next PRs - soon):
- rename package model to resourcemodel
- add generating MarshalJSON() function
- migrate resources to new config generation method (above needed first)
- improve unquoteDependsOnReferences method

TODOs (next PRs - later, left as next steps in readme):
- add config builders for other block types (Variable, Output, Localsl,
Module, Terraform)
- add provider to resource/datasource models (use in the
grant_ownership_acceptance_test)
- explore HCL v2 in more detail (especially struct tags generation;
probably with migration to plugin framework because of schema models)
- introduce some common interface for all three existing models
(ResourceModel, DatasourceModel, and ProviderModel)
- rename ResourceSchemaDetails (because it is used for the datasources
and provider too)
- consider duplicating the builders template from resource (currently
same template used for datasources and provider)
- consider merging ResourceModel with DatasourceModel (currently the
implementation is really similar)
  • Loading branch information
sfc-gh-asawicki authored Nov 29, 2024
1 parent 5d4ed3b commit 425787c
Show file tree
Hide file tree
Showing 51 changed files with 2,817 additions and 707 deletions.
16 changes: 14 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,20 @@ generate-resource-model-builders: ## Generate resource model builders
clean-resource-model-builders: ## Clean resource model builders
rm -f ./pkg/acceptance/bettertestspoc/config/model/*_gen.go

clean-all-assertions-and-config-models: clean-snowflake-object-assertions clean-snowflake-object-parameters-assertions clean-resource-assertions clean-resource-parameters-assertions clean-resource-show-output-assertions clean-resource-model-builders ## clean all generated assertions and config models
generate-provider-model-builders: ## Generate provider model builders
go generate ./pkg/acceptance/bettertestspoc/config/providermodel/generate.go

generate-all-assertions-and-config-models: generate-snowflake-object-assertions generate-snowflake-object-parameters-assertions generate-resource-assertions generate-resource-parameters-assertions generate-resource-show-output-assertions generate-resource-model-builders ## generate all assertions and config models
clean-provider-model-builders: ## Clean provider model builders
rm -f ./pkg/acceptance/bettertestspoc/config/providermodel/*_gen.go

generate-datasource-model-builders: ## Generate datasource model builders
go generate ./pkg/acceptance/bettertestspoc/config/datasourcemodel/generate.go

clean-datasource-model-builders: ## Clean datasource model builders
rm -f ./pkg/acceptance/bettertestspoc/config/datasourcemodel/*_gen.go

clean-all-assertions-and-config-models: clean-snowflake-object-assertions clean-snowflake-object-parameters-assertions clean-resource-assertions clean-resource-parameters-assertions clean-resource-show-output-assertions clean-resource-model-builders clean-provider-model-builders clean-datasource-model-builders ## clean all generated assertions and config models

generate-all-assertions-and-config-models: generate-snowflake-object-assertions generate-snowflake-object-parameters-assertions generate-resource-assertions generate-resource-parameters-assertions generate-resource-show-output-assertions generate-resource-model-builders generate-provider-model-builders generate-datasource-model-builders ## generate all assertions and config models

.PHONY: build-local clean-generator-poc dev-setup dev-cleanup docs docs-check fmt fmt-check fumpt help install lint lint-fix mod mod-check pre-push pre-push-check sweep test test-acceptance uninstall-tf
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/gookit/color v1.5.4
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/hcl v1.0.0
github.com/hashicorp/terraform-json v0.21.0
github.com/hashicorp/terraform-plugin-framework v1.8.0
github.com/hashicorp/terraform-plugin-framework-validators v0.12.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mO
github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/hashicorp/hc-install v0.6.3 h1:yE/r1yJvWbtrJ0STwScgEnCanb0U9v7zp0Gbkmcoxqs=
github.com/hashicorp/hc-install v0.6.3/go.mod h1:KamGdbodYzlufbWh4r9NRo8y6GLHWZP2GBtdnms1Ln0=
github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
github.com/hashicorp/hcl/v2 v2.19.1 h1://i05Jqznmb2EXqa39Nsvyan2o5XyMowW5fnCKW5RPI=
github.com/hashicorp/hcl/v2 v2.19.1/go.mod h1:ThLC89FV4p9MPW804KVbe/cEXoQ8NZEh+JtMeeGErHE=
github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y=
Expand Down
82 changes: 75 additions & 7 deletions pkg/acceptance/bettertestspoc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ It contains the following packages:
- resource parameters assertions (generated in subpackage `resourceparametersassert`)
- show output assertions (generated in subpackage `resourceshowoutputassert`)

- `config` - the new `ResourceModel` abstraction resides here. It provides models for objects and the builder methods allowing better config preparation in the acceptance tests.
- `config` - the new model abstractions (`ResourceModel`, `DatasourceModel`, and `ProviderModel`) reside here. They provide models for objects and the builder methods allowing better config preparation in the acceptance tests.
It aims to be more readable than using `Config:` with hardcoded string or `ConfigFile:` for file that is not directly reachable from the test body. Also, it should be easier to reuse the models and prepare convenience extension methods. The models are already generated.

## How it works
Expand Down Expand Up @@ -97,18 +97,58 @@ Resource config model builders can be generated automatically. For object `abc`
- add object you want to generate to `allResourceSchemaDefs` slice in the `assert/resourceassert/gen/resource_schema_def.go`
- to add custom (not generated) config builder methods create file `warehouse_model_ext` in the `config/model` package. Example would be:
```go
func BasicWarehouseModel(
func BasicAbcModel(
name string,
comment string,
) *WarehouseModel {
return WarehouseWithDefaultMeta(name).WithComment(comment)
) *AbcModel {
return AbcWithDefaultMeta(name).WithComment(comment)
}

func (w *WarehouseModel) WithWarehouseSizeEnum(warehouseSize sdk.WarehouseSize) *WarehouseModel {
func (w *AbcModel) WithWarehouseSizeEnum(warehouseSize sdk.WarehouseSize) *AbcModel {
return w.WithWarehouseSize(string(warehouseSize))
}
```

### Adding new datasource config model builders
Data source config model builders can be generated automatically. For object `abc` do the following:
- add object you want to generate to `allDatasourcesSchemaDefs` slice in the `config/datasourcemodel/gen/datasource_schema_def.go`
- to add custom (not generated) config builder methods create file `abc_model_ext` in the `config/datasourcemodel` package. Example would be:
```go
func BasicAbcModel(
name string,
comment string,
) *AbcModel {
return AbcWithDefaultMeta(name).WithComment(comment)
}

func (d *AbcModel) WithLimit(rows int) *AbcModel {
return d.WithLimitValue(
tfconfig.ObjectVariable(map[string]tfconfig.Variable{
"rows": tfconfig.IntegerVariable(rows),
}),
)
}
```

### Adding new provider config model builders
Provider config model builders can be generated automatically. For object `abc` do the following:
- add object you want to generate to `allProviderSchemaDefs` slice in the `config/providermodel/gen/provider_schema_def.go`
- to add custom (not generated) config builder methods create file `abc_model_ext` in the `config/providermodel` package. Example would be:
```go
func BasicAbcModel(
name string,
comment string,
) *AbcModel {
return AbcWithDefaultMeta(name).WithComment(comment)
}

func (w *AbcModel) WithWarehouseSizeEnum(warehouseSize sdk.WarehouseSize) *AbcModel {
return w.WithWarehouseSize(string(warehouseSize))
}
```

*Note*: our provider's config is already generated, so there should not be a need to generate any more providers (the regeneration or adding custom methods are still expected).

### Running the generators
Each of the above assertion types/config models has its own generator and cleanup entry in our Makefile.
You can generate config models with:
Expand Down Expand Up @@ -310,12 +350,12 @@ it will result in:
Test: TestInt_Warehouses/create:_complete
```

## Known limitations/planned improvements
## Planned improvements
- Test all the utilities for assertion/model construction (public interfaces, methods, functions).
- Verify if all the config types are supported.
- Consider a better implementation for the model conversion to config (TODO left in `config/config.go`).
- Support additional methods for references in models (TODO left in `config/config.go`).
- Support depends_on in models so that it can be chained like other resource fields (TODO left in `config/config.go`).
- Generate depends_on for all compatible models. Consider exporting it in meta (discussion: https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/3207#discussion_r1850053618).
- Add a convenience function to concatenate multiple models (TODO left in `config/config.go`).
- Add function to support using `ConfigFile:` in the acceptance tests (TODO left in `config/config.go`).
- Replace `acceptance/snowflakechecks` with the new proposed Snowflake objects assertions.
Expand Down Expand Up @@ -354,3 +394,31 @@ func (w *WarehouseDatasourceShowOutputAssert) IsEmpty() {
- utilize `ContainsExactlyInAnyOrder` function in `pkg/acceptance/bettertestspoc/assert/commons.go` to create asserts on collections that are order independent
- Additional asserts for sets and lists that wouldn't rely on the order of items saved to the state (SNOW-1706544)
- support generating provider config and use generated configs in `pkg/provider/provider_acceptance_test.go`
- add config builders for other block types (Variable, Output, Locals, Module, Terraform)
- add provider to resource/datasource models (use in the grant_ownership_acceptance_test)
- explore HCL v2 in more detail (especially struct tags generation; probably with migration to plugin framework because of schema models); ref: https://github.com/hashicorp/hcl/blob/bee2dc2e75f7528ad85777b7a013c13796426bd6/gohcl/encode_test.go#L48
- introduce some common interface for all three existing models (ResourceModel, DatasourceModel, and ProviderModel)
- rename ResourceSchemaDetails (because it is used for the datasources and provider too)
- consider duplicating the builders template from resource (currently same template used for datasources and provider which limits the customization possibilities for just one block type)
- consider merging ResourceModel with DatasourceModel (currently the implementation is really similar)
- remove schema.TypeMap workaround or make it wiser (e.g. during generation we could programmatically gather all schema.TypeMap and use this workaround only for them)

## Known limitations
- generating provider config may misbehave when used only with one object/map paramter (like `params`), e.g.:
```json
{
"provider": {
"snowflake": {
"params": {
"statement_timeout_in_seconds": 31337
}
}
}
}
```
will be converted to HCL:
```hcl
provider "snowflake" "params" {
statement_timeout_in_seconds = 31337
}
```
126 changes: 71 additions & 55 deletions pkg/acceptance/bettertestspoc/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,70 +9,95 @@ import (

tfconfig "github.com/hashicorp/terraform-plugin-testing/config"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/stretchr/testify/require"
)

// TODO [SNOW-1501905]: add possibility to have reference to another object (e.g. WithResourceMonitorReference); new config.Variable impl?
// TODO [SNOW-1501905]: generate With/SetDependsOn for the resources to preserve builder pattern
// TODO [SNOW-1501905]: add a convenience method to use multiple configs from multiple models

// ResourceModel is the base interface all of our config models will implement.
// To allow easy implementation, ResourceModelMeta can be embedded inside the struct (and the struct will automatically implement it).
type ResourceModel interface {
Resource() resources.Resource
ResourceName() string
SetResourceName(name string)
ResourceReference() string
DependsOn() []string
SetDependsOn(values ...string)
}
// ResourceFromModel should be used in terraform acceptance tests for Config attribute to get string config from ResourceModel.
// Current implementation is an improved implementation using two steps:
// - .tf.json generation
// - conversion to HCL using hcl v1 lib
// It is still not ideal. HCL v2 should be considered.
func ResourceFromModel(t *testing.T, model ResourceModel) string {
t.Helper()

type ResourceModelMeta struct {
name string
resource resources.Resource
dependsOn []string
}
resourceJson, err := DefaultJsonConfigProvider.ResourceJsonFromModel(model)
require.NoError(t, err)

func (m *ResourceModelMeta) Resource() resources.Resource {
return m.resource
}
hcl, err := DefaultHclConfigProvider.HclFromJson(resourceJson)
require.NoError(t, err)
t.Logf("Generated config:\n%s", hcl)

func (m *ResourceModelMeta) ResourceName() string {
return m.name
return hcl
}

func (m *ResourceModelMeta) SetResourceName(name string) {
m.name = name
}
// DatasourceFromModel should be used in terraform acceptance tests for Config attribute to get string config from DatasourceModel.
// Current implementation is an improved implementation using two steps:
// - .tf.json generation
// - conversion to HCL using hcl v1 lib
// It is still not ideal. HCL v2 should be considered.
func DatasourceFromModel(t *testing.T, model DatasourceModel) string {
t.Helper()

func (m *ResourceModelMeta) ResourceReference() string {
return fmt.Sprintf(`%s.%s`, m.resource, m.name)
}
datasourceJson, err := DefaultJsonConfigProvider.DatasourceJsonFromModel(model)
require.NoError(t, err)

func (m *ResourceModelMeta) DependsOn() []string {
return m.dependsOn
}
hcl, err := DefaultHclConfigProvider.HclFromJson(datasourceJson)
require.NoError(t, err)
t.Logf("Generated config:\n%s", hcl)

func (m *ResourceModelMeta) SetDependsOn(values ...string) {
m.dependsOn = values
return hcl
}

// DefaultResourceName is exported to allow assertions against the resources using the default name.
const DefaultResourceName = "test"
// ProviderFromModel should be used in terraform acceptance tests for Config attribute to get string config from ProviderModel.
// Current implementation is an improved implementation using two steps:
// - .tf.json generation
// - conversion to HCL using hcl v1 lib
// It is still not ideal. HCL v2 should be considered.
func ProviderFromModel(t *testing.T, model ProviderModel) string {
t.Helper()

providerJson, err := DefaultJsonConfigProvider.ProviderJsonFromModel(model)
require.NoError(t, err)

hcl, err := DefaultHclConfigProvider.HclFromJson(providerJson)
require.NoError(t, err)
hcl, err = revertEqualSignForMapTypeAttributes(hcl)
require.NoError(t, err)

func DefaultMeta(resource resources.Resource) *ResourceModelMeta {
return &ResourceModelMeta{name: DefaultResourceName, resource: resource}
return hcl
}

func Meta(resourceName string, resource resources.Resource) *ResourceModelMeta {
return &ResourceModelMeta{name: resourceName, resource: resource}
// FromModels allows to combine multiple models.
// TODO [SNOW-1501905]: introduce some common interface for all three existing models (ResourceModel, DatasourceModel, and ProviderModel)
func FromModels(t *testing.T, models ...any) string {
t.Helper()

var sb strings.Builder
for i, model := range models {
switch m := model.(type) {
case ResourceModel:
sb.WriteString(ResourceFromModel(t, m))
case DatasourceModel:
sb.WriteString(DatasourceFromModel(t, m))
case ProviderModel:
sb.WriteString(ProviderFromModel(t, m))
default:
t.Fatalf("unknown model: %T", model)
}
if i < len(models)-1 {
sb.WriteString("\n")
}
}
return sb.String()
}

// FromModel should be used in terraform acceptance tests for Config attribute to get string config from ResourceModel.
// Current implementation is really straightforward but it could be improved and tested. It may not handle all cases (like objects, lists, sets) correctly.
// TODO [SNOW-1501905]: use reflection to build config directly from model struct (or some other different way)
// TODO [SNOW-1501905]: add support for config.TestStepConfigFunc (to use as ConfigFile); the naive implementation would be to just create a tmp directory and save file there
// TODO [SNOW-1501905]: add generating MarshalJSON() function
// TODO [SNOW-1501905]: migrate resources to new config generation method (above needed first)
// Use ResourceFromModel, DatasourceFromModel, ProviderFromModel, and FromModels instead.
func FromModel(t *testing.T, model ResourceModel) string {
t.Helper()

Expand All @@ -99,7 +124,9 @@ func FromModel(t *testing.T, model ResourceModel) string {
return s
}

func FromModels(t *testing.T, models ...ResourceModel) string {
// FromModelsDeprecated allows to combine multiple resource models.
// Use FromModels instead.
func FromModelsDeprecated(t *testing.T, models ...ResourceModel) string {
t.Helper()
var sb strings.Builder
for _, model := range models {
Expand All @@ -110,6 +137,7 @@ func FromModels(t *testing.T, models ...ResourceModel) string {

// ConfigVariablesFromModel constructs config.Variables needed in acceptance tests that are using ConfigVariables in
// combination with ConfigDirectory. It's necessary for cases not supported by FromModel, like lists of objects.
// Use ResourceFromModel, DatasourceFromModel, ProviderFromModel, and FromModels instead.
func ConfigVariablesFromModel(t *testing.T, model ResourceModel) tfconfig.Variables {
t.Helper()
variables := make(tfconfig.Variables)
Expand Down Expand Up @@ -139,15 +167,3 @@ func ConfigVariablesFromModels(t *testing.T, variableName string, models ...Reso
variableName: tfconfig.ListVariable(allVariables...),
}
}

type nullVariable struct{}

// MarshalJSON returns the JSON encoding of nullVariable.
func (v nullVariable) MarshalJSON() ([]byte, error) {
return json.Marshal(nil)
}

// NullVariable returns nullVariable which implements Variable.
func NullVariable() nullVariable {
return nullVariable{}
}
Loading

0 comments on commit 425787c

Please sign in to comment.