Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,20 @@ 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]
}

// 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
Expand All @@ -105,4 +106,3 @@ output "id" {
output "image" {
value = envbuilder_cached_image.example.image
}

42 changes: 26 additions & 16 deletions internal/provider/cached_image_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -631,34 +632,43 @@ 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
// tfValueToString converts an attr.Value to its string representation
// 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.(interface{ ValueString() string }); ok {
return vs.ValueString()
}
if vb, ok := val.(interface{ ValueBool() bool }); ok {
return fmt.Sprintf("%t", vb.ValueBool())
}
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))
}

func appendKnownEnvToList(list types.List, key string, value stringable) types.List {
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
}
Expand Down
11 changes: 6 additions & 5 deletions internal/provider/cached_image_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand Down