From 75314255e2767d2d66d325349752e92cd3320dce Mon Sep 17 00:00:00 2001 From: KK <68334452+healthjyk@users.noreply.github.com> Date: Tue, 12 Dec 2023 20:19:16 +0800 Subject: [PATCH] refactor: update the struct of TerraformConfig (#670) --- pkg/apis/workspace/types.go | 17 ++++- .../testdata/workspaces/for_delete_ws.yaml | 2 +- .../testdata/workspaces/for_update_ws.yaml | 2 +- pkg/workspace/util.go | 6 +- pkg/workspace/util_test.go | 14 ++-- pkg/workspace/validation.go | 36 ++++++++-- pkg/workspace/validation_test.go | 70 +++++++++++++++++-- 7 files changed, 123 insertions(+), 24 deletions(-) diff --git a/pkg/apis/workspace/types.go b/pkg/apis/workspace/types.go index 9b74ac48..ca16ec93 100644 --- a/pkg/apis/workspace/types.go +++ b/pkg/apis/workspace/types.go @@ -81,7 +81,22 @@ type KubernetesConfig struct { // TerraformConfig contains the config of multiple terraform provider config, whose key is // the provider name. -type TerraformConfig map[string]GenericConfig +type TerraformConfig map[string]*ProviderConfig + +// ProviderConfig contains the full configurations of a specified provider. It is the combination +// of the specified provider's config in blocks "terraform/required_providers" and "providers" in +// terraform hcl file, where the former is described by fields Source and Version, and the latter +// is described by GenericConfig cause different provider has different config. +type ProviderConfig struct { + // Source of the provider. + Source string `yaml:"source" json:"source"` + + // Version of the provider. + Version string `yaml:"version" json:"version"` + + // GenericConfig is used to describe the config of a specified terraform provider. + GenericConfig `yaml:",inline,omitempty" json:",inline,omitempty"` +} // BackendConfigs contains config of the backend, which is used to store state, etc. Only one kind // backend can be configured. diff --git a/pkg/workspace/testdata/workspaces/for_delete_ws.yaml b/pkg/workspace/testdata/workspaces/for_delete_ws.yaml index d0864650..2334cf44 100644 --- a/pkg/workspace/testdata/workspaces/for_delete_ws.yaml +++ b/pkg/workspace/testdata/workspaces/for_delete_ws.yaml @@ -17,8 +17,8 @@ runtimes: kubeConfig: /etc/kubeconfig.yaml terraform: aws: - region: us-east-1 source: hashicorp/aws version: 1.0.4 + region: us-east-1 backends: local: {} diff --git a/pkg/workspace/testdata/workspaces/for_update_ws.yaml b/pkg/workspace/testdata/workspaces/for_update_ws.yaml index d0864650..2334cf44 100644 --- a/pkg/workspace/testdata/workspaces/for_update_ws.yaml +++ b/pkg/workspace/testdata/workspaces/for_update_ws.yaml @@ -17,8 +17,8 @@ runtimes: kubeConfig: /etc/kubeconfig.yaml terraform: aws: - region: us-east-1 source: hashicorp/aws version: 1.0.4 + region: us-east-1 backends: local: {} diff --git a/pkg/workspace/util.go b/pkg/workspace/util.go index 8f3b99f9..20231ff4 100644 --- a/pkg/workspace/util.go +++ b/pkg/workspace/util.go @@ -160,10 +160,10 @@ func GetTerraformConfig(configs *workspace.RuntimeConfigs) (workspace.TerraformC return configs.Terraform, nil } -// GetTerraformProviderConfig returns the specified terraform provider config from runtime config, should -// be called after ValidateRuntimeConfigs. +// GetProviderConfig returns the specified terraform provider config from runtime config, should be called +// after ValidateRuntimeConfigs. // If got empty terraform config, ErrEmptyTerraformProviderConfig will get returned. -func GetTerraformProviderConfig(configs *workspace.RuntimeConfigs, providerName string) (workspace.GenericConfig, error) { +func GetProviderConfig(configs *workspace.RuntimeConfigs, providerName string) (*workspace.ProviderConfig, error) { if providerName == "" { return nil, ErrEmptyTerraformProviderName } diff --git a/pkg/workspace/util_test.go b/pkg/workspace/util_test.go index bd1ff5b0..14bd6e15 100644 --- a/pkg/workspace/util_test.go +++ b/pkg/workspace/util_test.go @@ -120,17 +120,19 @@ func Test_GetTerraformProviderConfig(t *testing.T) { name string success bool providerName string - providerConfig workspace.GenericConfig + providerConfig *workspace.ProviderConfig runtimeConfigs *workspace.RuntimeConfigs }{ { name: "successfully get terraform provider config", success: true, providerName: "aws", - providerConfig: workspace.GenericConfig{ - "version": "1.0.4", - "source": "hashicorp/aws", - "region": "us-east-1", + providerConfig: &workspace.ProviderConfig{ + Source: "hashicorp/aws", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "region": "us-east-1", + }, }, runtimeConfigs: mockValidRuntimeConfigs(), }, @@ -145,7 +147,7 @@ func Test_GetTerraformProviderConfig(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - cfg, err := GetTerraformProviderConfig(tc.runtimeConfigs, tc.providerName) + cfg, err := GetProviderConfig(tc.runtimeConfigs, tc.providerName) assert.Equal(t, tc.success, err == nil) assert.Equal(t, tc.providerConfig, cfg) }) diff --git a/pkg/workspace/validation.go b/pkg/workspace/validation.go index 62fde6bd..cfb90f22 100644 --- a/pkg/workspace/validation.go +++ b/pkg/workspace/validation.go @@ -20,9 +20,13 @@ var ( ErrRepeatedModuleConfigSelectedProjects = errors.New("project should not repeat in one patcher block's projectSelector") ErrMultipleModuleConfigSelectedProjects = errors.New("a project cannot assign in more than one patcher block's projectSelector") - ErrEmptyKubeConfig = errors.New("empty kubeconfig") - ErrEmptyTerraformProviderName = errors.New("empty terraform provider name") - ErrEmptyTerraformProviderConfig = errors.New("empty terraform provider config") + ErrEmptyKubeConfig = errors.New("empty kubeconfig") + ErrEmptyTerraformProviderName = errors.New("empty terraform provider name") + ErrEmptyTerraformProviderConfig = errors.New("empty terraform provider config") + ErrEmptyTerraformProviderSource = errors.New("empty provider source") + ErrEmptyTerraformProviderVersion = errors.New("empty provider version") + ErrEmptyTerraformProviderConfigKey = errors.New("empty provider config key") + ErrEmptyTerraformProviderConfigValue = errors.New("empty provider config value") ErrMultipleBackends = errors.New("more than one backend configured") ErrEmptyMysqlDBName = errors.New("empty db name") @@ -159,8 +163,30 @@ func ValidateTerraformConfig(config workspace.TerraformConfig) error { if name == "" { return ErrEmptyTerraformProviderName } - if len(cfg) == 0 { - return ErrEmptyTerraformProviderConfig + if cfg == nil { + return fmt.Errorf("%w of provider %s", ErrEmptyTerraformProviderConfig, name) + } + if err := ValidateProviderConfig(cfg); err != nil { + return fmt.Errorf("invalid terraform provider %s: %w", name, err) + } + } + return nil +} + +// ValidateProviderConfig is used to validate the providerConfig is valid or not. +func ValidateProviderConfig(config *workspace.ProviderConfig) error { + if config.Source == "" { + return ErrEmptyTerraformProviderSource + } + if config.Version == "" { + return ErrEmptyTerraformProviderVersion + } + for k, v := range config.GenericConfig { + if k == "" { + return ErrEmptyTerraformProviderConfigKey + } + if v == nil { + return fmt.Errorf("%w of field %s", ErrEmptyTerraformProviderConfigValue, k) } } return nil diff --git a/pkg/workspace/validation_test.go b/pkg/workspace/validation_test.go index 0d6fb82d..18dd5347 100644 --- a/pkg/workspace/validation_test.go +++ b/pkg/workspace/validation_test.go @@ -141,9 +141,11 @@ func mockValidKubernetesConfig() *workspace.KubernetesConfig { func mockValidTerraformConfig() workspace.TerraformConfig { return workspace.TerraformConfig{ "aws": { - "version": "1.0.4", - "source": "hashicorp/aws", - "region": "us-east-1", + Source: "hashicorp/aws", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "region": "us-east-1", + }, }, } } @@ -346,9 +348,11 @@ func TestValidateTerraformConfig(t *testing.T) { success: false, terraformConfig: workspace.TerraformConfig{ "": { - "version": "1.0.4", - "source": "hashicorp/aws", - "region": "us-east-1", + Source: "hashicorp/aws", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "region": "us-east-1", + }, }, }, }, @@ -356,7 +360,59 @@ func TestValidateTerraformConfig(t *testing.T) { name: "invalid terraform config empty provider config", success: false, terraformConfig: workspace.TerraformConfig{ - "aws": {}, + "aws": nil, + }, + }, + { + name: "invalid terraform config empty provider source", + success: false, + terraformConfig: workspace.TerraformConfig{ + "aws": { + Source: "", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "region": "us-east-1", + }, + }, + }, + }, + { + name: "invalid terraform config empty provider version", + success: false, + terraformConfig: workspace.TerraformConfig{ + "aws": { + Source: "hashicorp/aws", + Version: "", + GenericConfig: workspace.GenericConfig{ + "region": "us-east-1", + }, + }, + }, + }, + { + name: "invalid terraform config empty provider config key", + success: false, + terraformConfig: workspace.TerraformConfig{ + "aws": { + Source: "hashicorp/aws", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "": "us-east-1", + }, + }, + }, + }, + { + name: "invalid terraform config empty provider config value", + success: false, + terraformConfig: workspace.TerraformConfig{ + "aws": { + Source: "hashicorp/aws", + Version: "1.0.4", + GenericConfig: workspace.GenericConfig{ + "region": nil, + }, + }, }, }, }