From 9f22057053959ef9560211426dd7922314f71619 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 14 Aug 2024 14:13:14 +0100 Subject: [PATCH 1/3] chore(examples): update envbuilder_cached_image example --- .../envbuilder_cached_image_resource.tf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/resources/envbuilder_cached_image/envbuilder_cached_image_resource.tf b/examples/resources/envbuilder_cached_image/envbuilder_cached_image_resource.tf index 304aea4..af90219 100644 --- a/examples/resources/envbuilder_cached_image/envbuilder_cached_image_resource.tf +++ b/examples/resources/envbuilder_cached_image/envbuilder_cached_image_resource.tf @@ -66,10 +66,11 @@ resource "envbuilder_cached_image" "example" { builder_image = var.builder_image git_url = var.repo_url cache_repo = local.cache_repo + insecure = true extra_env = { "ENVBUILDER_VERBOSE" : "true" "ENVBUILDER_INSECURE" : "true" # due to local registry - "ENVBUILDER_INIT_SCRIPT" : "sleep infinity" + "ENVBUILDER_INIT_SCRIPT" : "#!/usr/bin/env bash\necho Hello && sleep infinity" "ENVBUILDER_PUSH_IMAGE" : "true" } depends_on = [docker_container.registry] @@ -77,8 +78,8 @@ resource "envbuilder_cached_image" "example" { // Run the cached image. Depending on the contents of // the cache repo, this will either be var.builder_image -// or a previously built image pusehd to var.cache_repo. -// Running `terraform apply` once (assuming empty cache) +// or a previously built image pushed to var.cache_repo. +// Running `terraform apply` once (assuming empty cache) // will result in the builder image running, and the built // image being pushed to the cache repo. // Running `terraform apply` again will result in the @@ -105,4 +106,3 @@ output "id" { output "image" { value = envbuilder_cached_image.example.image } - From 93f1e0f016841468ede664d95d588f2380f16ddc Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 14 Aug 2024 14:16:01 +0100 Subject: [PATCH 2/3] fix(internal/provider): correct escaping of strings in envbuilder_cached_image.env --- internal/provider/cached_image_resource.go | 55 +++++++++++++------ .../provider/cached_image_resource_test.go | 11 ++-- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/internal/provider/cached_image_resource.go b/internal/provider/cached_image_resource.go index e165453..49e1508 100644 --- a/internal/provider/cached_image_resource.go +++ b/internal/provider/cached_image_resource.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/uuid" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" @@ -631,20 +632,48 @@ func extractEnvbuilderFromImage(ctx context.Context, imgRef, destPath string) er return fmt.Errorf("extract envbuilder binary from image %q: %w", imgRef, os.ErrNotExist) } -// NOTE: the String() method of Terraform values will evalue to `` if unknown. -// Check IsUnknown() first before calling String(). -type stringable interface { - IsUnknown() bool - IsNull() bool - String() string +// The below interfaces are to handle converting Terraform types can be converted to a string. +// The String() method on an attr.Value creates a 'human-readable' version of the type, which +// leads to quotes, escaped characters, and other assorted sadness. +type valuestringer interface { + ValueString() string } -func appendKnownEnvToList(list types.List, key string, value stringable) types.List { +type valuebooler interface { + ValueBool() bool +} + +type valueint64er interface { + ValueInt64() int64 +} + +// tfValueToString converts an attr.Value to its string representation +// based on its implementation of the above interfaces. +func tfValueToString(val attr.Value) string { + if val.IsUnknown() || val.IsNull() { + return "" + } + if vs, ok := val.(valuestringer); ok { + return vs.ValueString() + } + if vb, ok := val.(valuebooler); ok { + return fmt.Sprintf("%t", vb.ValueBool()) + } + if vi, ok := val.(valueint64er); ok { + return fmt.Sprintf("%d", vi.ValueInt64()) + } + panic(fmt.Errorf("tfValueToString: value %T is not a supported type", val)) +} + +func appendKnownEnvToList(list types.List, key string, value attr.Value) types.List { if value.IsUnknown() || value.IsNull() { return list } - val := strings.Trim(value.String(), `"`) - elem := types.StringValue(fmt.Sprintf("%s=%s", key, val)) + var sb strings.Builder + _, _ = sb.WriteString(key) + _, _ = sb.WriteRune('=') + _, _ = sb.WriteString(tfValueToString(value)) + elem := types.StringValue(sb.String()) list, _ = types.ListValue(types.StringType, append(list.Elements(), elem)) return list } @@ -652,13 +681,7 @@ func appendKnownEnvToList(list types.List, key string, value stringable) types.L func tfListToStringSlice(l types.List) []string { var ss []string for _, el := range l.Elements() { - if sv, ok := el.(stringable); !ok { - panic(fmt.Sprintf("developer error: element %+v must be stringable", el)) - } else if sv.IsUnknown() { - ss = append(ss, "") - } else { - ss = append(ss, sv.String()) - } + ss = append(ss, tfValueToString(el)) } return ss } diff --git a/internal/provider/cached_image_resource_test.go b/internal/provider/cached_image_resource_test.go index 30bfc0e..4653f8b 100644 --- a/internal/provider/cached_image_resource_test.go +++ b/internal/provider/cached_image_resource_test.go @@ -20,7 +20,8 @@ func TestAccCachedImageResource(t *testing.T) { } deps := setup(ctx, t, files) - deps.ExtraEnv["FOO"] = "bar" + deps.ExtraEnv["FOO"] = `bar +baz` // THIS IS A LOAD-BEARING NEWLINE. DO NOT REMOVE. resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ @@ -42,7 +43,7 @@ func TestAccCachedImageResource(t *testing.T) { resource.TestCheckResourceAttr("envbuilder_cached_image.test", "image", deps.BuilderImage), // Inputs should still be present. resource.TestCheckResourceAttr("envbuilder_cached_image.test", "cache_repo", deps.CacheRepo), - resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar"), + resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar\nbaz"), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "git_url", deps.Repo.URL), // Should be empty resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"), @@ -63,7 +64,7 @@ func TestAccCachedImageResource(t *testing.T) { resource.TestCheckResourceAttr("envbuilder_cached_image.test", "image", deps.BuilderImage), // Inputs should still be present. resource.TestCheckResourceAttr("envbuilder_cached_image.test", "cache_repo", deps.CacheRepo), - resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar"), + resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar\nbaz"), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "git_url", deps.Repo.URL), // Should be empty resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"), @@ -81,7 +82,7 @@ func TestAccCachedImageResource(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( // Inputs should still be present. resource.TestCheckResourceAttr("envbuilder_cached_image.test", "cache_repo", deps.CacheRepo), - resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar"), + resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar\nbaz"), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "git_url", deps.Repo.URL), // Should be empty resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"), @@ -92,7 +93,7 @@ func TestAccCachedImageResource(t *testing.T) { resource.TestCheckResourceAttr("envbuilder_cached_image.test", "exists", "true"), resource.TestCheckResourceAttrSet("envbuilder_cached_image.test", "image"), resource.TestCheckResourceAttrWith("envbuilder_cached_image.test", "image", quotedPrefix(deps.CacheRepo)), - resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.0", "FOO=bar"), + resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.0", "FOO=bar\nbaz"), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.1", fmt.Sprintf("ENVBUILDER_CACHE_REPO=%s", deps.CacheRepo)), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.2", fmt.Sprintf("ENVBUILDER_GIT_URL=%s", deps.Repo.URL)), resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.3", "ENVBUILDER_REMOTE_REPO_BUILD_MODE=true"), From 8d8277ae660fa7cdf882c75dacfbbff6ded22a3c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 14 Aug 2024 14:27:34 +0100 Subject: [PATCH 3/3] inline single-method interfaces --- internal/provider/cached_image_resource.go | 25 ++++++---------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/internal/provider/cached_image_resource.go b/internal/provider/cached_image_resource.go index 49e1508..6505736 100644 --- a/internal/provider/cached_image_resource.go +++ b/internal/provider/cached_image_resource.go @@ -632,34 +632,21 @@ func extractEnvbuilderFromImage(ctx context.Context, imgRef, destPath string) er return fmt.Errorf("extract envbuilder binary from image %q: %w", imgRef, os.ErrNotExist) } -// The below interfaces are to handle converting Terraform types can be converted to a string. -// The String() method on an attr.Value creates a 'human-readable' version of the type, which -// leads to quotes, escaped characters, and other assorted sadness. -type valuestringer interface { - ValueString() string -} - -type valuebooler interface { - ValueBool() bool -} - -type valueint64er interface { - ValueInt64() int64 -} - // tfValueToString converts an attr.Value to its string representation -// based on its implementation of the above interfaces. +// based on its Terraform type. This is needed because the String() +// method on an attr.Value creates a 'human-readable' version of the type, which +// leads to quotes, escaped characters, and other assorted sadness. func tfValueToString(val attr.Value) string { if val.IsUnknown() || val.IsNull() { return "" } - if vs, ok := val.(valuestringer); ok { + if vs, ok := val.(interface{ ValueString() string }); ok { return vs.ValueString() } - if vb, ok := val.(valuebooler); ok { + if vb, ok := val.(interface{ ValueBool() bool }); ok { return fmt.Sprintf("%t", vb.ValueBool()) } - if vi, ok := val.(valueint64er); ok { + if vi, ok := val.(interface{ ValueInt64() int64 }); ok { return fmt.Sprintf("%d", vi.ValueInt64()) } panic(fmt.Errorf("tfValueToString: value %T is not a supported type", val))