fix(internal/provider): correct escaping of strings in envbuilder_cached_image.env

Author: johnstcn

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,34 +632,56 @@ 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 `<null>` 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
 }
 
 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
 }

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"),