From 629e921eaa467b50851841328c93cce323fa28f2 Mon Sep 17 00:00:00 2001 From: Rohith Jayawardene Date: Tue, 3 Sep 2024 13:42:39 +0100 Subject: [PATCH] [FEATURE] - Provider JSON (#1504) * [FEATURE] - Provider JSON Switching the rendering of the provider.tf to json in order to get around the configuration issues highlighted in https://github.com/appvia/terranetes-controller/issues/1483 * fix: there was an issue with formatting * fix: removing the typo from the unit tests * fix: forgot to change the name of the provider file in the template * build: allowing for the infracosts to be optional, as they break on the mac due to an lack of arm64 images * fix: we were missing the ending bracket in the json * build(ci): spliting out the formatting into another job * fix: needed to fix up the unit test now the extra bracket is working * build: Revert "build: allowing for the infracosts to be optional, as they break on the mac due to an lack of arm64 images" This reverts commit b1eb41430c70feebf6341b698ea899d8b8a1d19e. --- .github/workflows/ci.yaml | 16 +++++++- .../terraform/v1alpha1/configuration_types.go | 2 +- pkg/assets/job.yaml.tpl | 4 +- .../configuration/reconcile_test.go | 4 +- pkg/utils/convert.go | 11 ++++- pkg/utils/convert_test.go | 12 ++++++ pkg/utils/terraform/utils.go | 23 +++++++---- pkg/utils/terraform/utils_test.go | 40 +++++++++++++------ 8 files changed, 84 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c604de96b..20ef9b71a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -69,10 +69,22 @@ jobs: go-version: "1.22" - name: Linting run: | - make check-gofmt - make shfmt make golangci-lint + formating: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: "1.22" + - name: Format + run: | + make gofmt + make shfmt + helm: runs-on: ubuntu-latest steps: diff --git a/pkg/apis/terraform/v1alpha1/configuration_types.go b/pkg/apis/terraform/v1alpha1/configuration_types.go index 798d6f14b..6bdadd037 100644 --- a/pkg/apis/terraform/v1alpha1/configuration_types.go +++ b/pkg/apis/terraform/v1alpha1/configuration_types.go @@ -110,7 +110,7 @@ const ( // TerraformTFVarsConfigMapKey is the key name for any TFVars file in the configmap TerraformTFVarsConfigMapKey = "variables.tfvars" // TerraformProviderConfigMapKey is the key name for the terraform variables in the configmap - TerraformProviderConfigMapKey = "provider.tf" + TerraformProviderConfigMapKey = "provider.tf.json" // TerraformJobTemplateConfigMapKey is the key name for the job template in the configmap TerraformJobTemplateConfigMapKey = "job.yaml" ) diff --git a/pkg/assets/job.yaml.tpl b/pkg/assets/job.yaml.tpl index 8ac8dfe7c..2dc3ea0ae 100644 --- a/pkg/assets/job.yaml.tpl +++ b/pkg/assets/job.yaml.tpl @@ -51,8 +51,8 @@ spec: items: - key: backend.tf path: backend.tf - - key: provider.tf - path: provider.tf + - key: provider.tf.json + path: provider.tf.json {{- if .EnableVariables }} - key: variables.tfvars.json path: variables.tfvars.json diff --git a/pkg/controller/configuration/reconcile_test.go b/pkg/controller/configuration/reconcile_test.go index 1793d0f90..17d728439 100644 --- a/pkg/controller/configuration/reconcile_test.go +++ b/pkg/controller/configuration/reconcile_test.go @@ -1956,8 +1956,8 @@ terraform { Expect(backend).To(Equal(expected)) }) - It("should have a provider.tf", func() { - expected := "provider \"aws\" {\n}\n" + It("should have a provider.tf.json", func() { + expected := "{\n \"provider\": {\n \"aws\": {}\n }\n}\n" secret := &v1.Secret{} secret.Namespace = ctrl.ControllerNamespace secret.Name = configuration.GetTerraformConfigSecretName() diff --git a/pkg/utils/convert.go b/pkg/utils/convert.go index 6a208d4a5..fb0198aa7 100644 --- a/pkg/utils/convert.go +++ b/pkg/utils/convert.go @@ -1,6 +1,10 @@ package utils -import "fmt" +import ( + "fmt" + + "github.com/tidwall/pretty" +) // ByteCountSI returns the number of bytes in the given number of SI units. func ByteCountSI(b int64) string { @@ -18,3 +22,8 @@ func ByteCountSI(b int64) string { return fmt.Sprintf("%.1f%cB", float64(b)/float64(div), "kMGTPE"[exp]) } + +// PrettyJSON returns a pretty-printed version of the given JSON data. +func PrettyJSON(data []byte) []byte { + return pretty.Pretty(data) +} diff --git a/pkg/utils/convert_test.go b/pkg/utils/convert_test.go index 617889bf1..526e443ee 100644 --- a/pkg/utils/convert_test.go +++ b/pkg/utils/convert_test.go @@ -36,3 +36,15 @@ func TestBytesCountSI(t *testing.T) { assert.Equal(t, c.Expected, ByteCountSI(c.Bytes)) } } + +func TestPrettyJSON(t *testing.T) { + cases := []struct { + Data []byte + Expected []byte + }{ + {[]byte(`{"foo":"bar"}`), []byte("{\n \"foo\": \"bar\"\n}\n")}, + } + for _, c := range cases { + assert.Equal(t, string(c.Expected), string(PrettyJSON(c.Data))) + } +} diff --git a/pkg/utils/terraform/utils.go b/pkg/utils/terraform/utils.go index ac60465c3..4a1512a90 100644 --- a/pkg/utils/terraform/utils.go +++ b/pkg/utils/terraform/utils.go @@ -26,6 +26,7 @@ import ( "io" terraformv1alpha1 "github.com/appvia/terranetes-controller/pkg/apis/terraform/v1alpha1" + "github.com/appvia/terranetes-controller/pkg/utils" "github.com/appvia/terranetes-controller/pkg/utils/template" ) @@ -78,12 +79,15 @@ terraform { ` // providerTF is a template for a terraform provider -var providerTF = `provider "{{ .provider }}" { -{{- if .configuration }} - {{ toHCL .configuration | nindent 2 }} -{{- end }} -} -` +var providerTF = `{ + "provider": { + "{{ .provider }}": { + {{- if .configuration }} + {{ toJson .configuration }} + {{- end }} + } + } +}` // Decode returns a Reader that will decode a gzip byte stream func Decode(data []byte) (io.Reader, error) { @@ -155,10 +159,15 @@ func NewTerraformProvider(provider string, configuration []byte) ([]byte, error) } } - return Template(providerTF, map[string]interface{}{ + rendered, err := Template(providerTF, map[string]interface{}{ "configuration": config, "provider": provider, }) + if err != nil { + return nil, err + } + + return utils.PrettyJSON(rendered), nil } // BackendOptions are the options used to generate the backend diff --git a/pkg/utils/terraform/utils_test.go b/pkg/utils/terraform/utils_test.go index 33c3237a4..3c4258275 100644 --- a/pkg/utils/terraform/utils_test.go +++ b/pkg/utils/terraform/utils_test.go @@ -69,14 +69,14 @@ terraform { func TestNewTerraformProvider(t *testing.T) { azureConfig := ` -{ - "use_oidc": true, - "storage_use_azuread": true, - "subscription_id": "injected", - "tenant_id": "injected", - "client_id": "injected", - "oidc_token_file_path": "/var/run/secrets/azure/tokens/azure-identity-token" -}` + { + "use_oidc": true, + "storage_use_azuread": true, + "subscription_id": "injected", + "tenant_id": "injected", + "client_id": "injected", + "oidc_token_file_path": "/var/run/secrets/azure/tokens/azure-identity-token" + }` cases := []struct { Provider *terraformv1alpha1.Provider @@ -87,35 +87,49 @@ func TestNewTerraformProvider(t *testing.T) { Provider: terraformv1alpha1.AWSProviderType, Configuration: nil, }}, - Expected: "provider \"aws\" {\n}\n", + Expected: "{\n \"provider\": {\n \"aws\": {}\n }\n}\n", + }, + { + Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ + Provider: terraformv1alpha1.AWSProviderType, + Configuration: &runtime.RawExtension{Raw: []byte("{}")}, + }}, + Expected: "{\n \"provider\": {\n \"aws\": {}\n }\n}\n", + }, + { + Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ + Provider: terraformv1alpha1.AWSProviderType, + Configuration: &runtime.RawExtension{Raw: []byte(`{"default_tags": { "tags": { "hello": "world" }}}`)}, + }}, + Expected: "{\n \"provider\": {\n \"aws\": {\n \"default_tags\": {\n \"tags\": {\n \"hello\": \"world\"\n }\n }\n }\n }\n}\n", }, { Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ Provider: terraformv1alpha1.AzureProviderType, Configuration: nil, }}, - Expected: "provider \"azurerm\" {\n \n features {}\n \n}\n", + Expected: "{\n \"provider\": {\n \"azurerm\": {\n \"features\": {}\n }\n }\n}\n", }, { Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ Provider: terraformv1alpha1.AzureProviderType, Configuration: &runtime.RawExtension{Raw: []byte("{\"features\": {\"hello\": \"world\"}}")}, }}, - Expected: "provider \"azurerm\" {\n \n features {\n hello = \"world\"\n }\n \n}\n", + Expected: "{\n \"provider\": {\n \"azurerm\": {\n \"features\": {\n \"hello\": \"world\"\n }\n }\n }\n}\n", }, { Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ Provider: terraformv1alpha1.AzureProviderType, Configuration: &runtime.RawExtension{Raw: []byte("{\"features\": \"hello\"}}")}, }}, - Expected: "provider \"azurerm\" {\n \n features = \"hello\"\n \n}\n", + Expected: "{\n \"provider\": {\n \"azurerm\": {\n \"features\": \"hello\"\n }\n }\n}\n", }, { Provider: &terraformv1alpha1.Provider{Spec: terraformv1alpha1.ProviderSpec{ Provider: terraformv1alpha1.AzureProviderType, Configuration: &runtime.RawExtension{Raw: []byte(azureConfig)}, }}, - Expected: "provider \"azurerm\" {\n \n client_id = \"injected\"\n \n features {}\n \n oidc_token_file_path = \"/var/run/secrets/azure/tokens/azure-identity-token\"\n \n storage_use_azuread = true\n \n subscription_id = \"injected\"\n \n tenant_id = \"injected\"\n \n use_oidc = true\n \n}\n", + Expected: "{\n \"provider\": {\n \"azurerm\": {\n \"client_id\": \"injected\",\n \"features\": {},\n \"oidc_token_file_path\": \"/var/run/secrets/azure/tokens/azure-identity-token\",\n \"storage_use_azuread\": true,\n \"subscription_id\": \"injected\",\n \"tenant_id\": \"injected\",\n \"use_oidc\": true\n }\n }\n}\n", }, }