feat(internal/provider): add env_map to cached_image_resource (#37)

This PR adds `env_map` to `cached_image_resource.` This consists of the computed env in map format, which can be useful for other providers that do not expect `KEY=VALUE` format.

Author: johnstcn

docs/resources/cached_image.md

@@ -48,7 +48,8 @@ The cached image resource can be used to retrieve a cached image produced by env
 
 ### Read-Only
 
-- `env` (List of String, Sensitive) Computed envbuilder configuration to be set for the container. May contain secrets.
+- `env` (List of String, Sensitive) Computed envbuilder configuration to be set for the container in the form of a list of strings of `key=value`. May contain secrets.
+- `env_map` (Map of String, Sensitive) Computed envbuilder configuration to be set for the container in the form of a key-value map. May contain secrets.
 - `exists` (Boolean) Whether the cached image was exists or not for the given config.
 - `id` (String) Cached image identifier. This will generally be the image's SHA256 digest.
 - `image` (String) Outputs the cached image repo@digest if it exists, and builder image otherwise.

internal/provider/cached_image_resource.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	kconfig "github.com/GoogleContainerTools/kaniko/pkg/config"
@@ -23,6 +24,7 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/hashicorp/terraform-plugin-framework/attr"
+	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
@@ -31,6 +33,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 )
 
@@ -77,6 +80,7 @@ type CachedImageResourceModel struct {
 	WorkspaceFolder      types.String `tfsdk:"workspace_folder"`
 	// Computed "outputs".
 	Env    types.List   `tfsdk:"env"`
+	EnvMap types.Map    `tfsdk:"env_map"`
 	Exists types.Bool   `tfsdk:"exists"`
 	ID     types.String `tfsdk:"id"`
 	Image  types.String `tfsdk:"image"`
@@ -226,16 +230,24 @@ func (r *CachedImageResource) Schema(ctx context.Context, req resource.SchemaReq
 			},
 
 			// Computed "outputs".
-			// TODO(mafredri): Map vs List? Support both?
 			"env": schema.ListAttribute{
-				MarkdownDescription: "Computed envbuilder configuration to be set for the container. May contain secrets.",
+				MarkdownDescription: "Computed envbuilder configuration to be set for the container in the form of a list of strings of `key=value`. May contain secrets.",
 				ElementType:         types.StringType,
 				Computed:            true,
 				Sensitive:           true,
 				PlanModifiers: []planmodifier.List{
 					listplanmodifier.RequiresReplace(),
 				},
 			},
+			"env_map": schema.MapAttribute{
+				MarkdownDescription: "Computed envbuilder configuration to be set for the container in the form of a key-value map. May contain secrets.",
+				ElementType:         types.StringType,
+				Computed:            true,
+				Sensitive:           true,
+				PlanModifiers: []planmodifier.Map{
+					mapplanmodifier.RequiresReplace(),
+				},
+			},
 			"exists": schema.BoolAttribute{
 				MarkdownDescription: "Whether the cached image was exists or not for the given config.",
 				Computed:            true,
@@ -338,28 +350,36 @@ func (r *CachedImageResource) Read(ctx context.Context, req resource.ReadRequest
 	data.Exists = types.BoolValue(true)
 
 	// Set the expected environment variables.
+	env := make(map[string]string)
 	for key, elem := range data.ExtraEnv.Elements() {
-		data.Env = appendKnownEnvToList(data.Env, key, elem)
+		env[key] = tfValueToString(elem)
 	}
 
-	data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_CACHE_REPO", data.CacheRepo)
-	data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_URL", data.GitURL)
+	env["ENVBUILDER_CACHE_REPO"] = tfValueToString(data.CacheRepo)
+	env["ENVBUILDER_GIT_URL"] = tfValueToString(data.GitURL)
+
 	if !data.CacheTTLDays.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_CACHE_TTL_DAYS", data.CacheTTLDays)
+		env["ENVBUILDER_CACHE_TTL_DAYS"] = tfValueToString(data.CacheTTLDays)
 	}
 	if !data.GitUsername.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_USERNAME", data.GitUsername)
+		env["ENVBUILDER_GIT_USERNAME"] = tfValueToString(data.GitUsername)
 	}
 	if !data.GitPassword.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_PASSWORD", data.GitPassword)
+		env["ENVBUILDER_GIT_PASSWORD"] = tfValueToString(data.GitPassword)
 	}
 	// Default to remote build mode.
 	if data.RemoteRepoBuildMode.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_REMOTE_REPO_BUILD_MODE", types.BoolValue(true))
+		env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = "true"
 	} else {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_REMOTE_REPO_BUILD_MODE", data.RemoteRepoBuildMode)
+		env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = tfValueToString(data.RemoteRepoBuildMode)
 	}
 
+	var diag diag.Diagnostics
+	data.EnvMap, diag = basetypes.NewMapValueFrom(ctx, types.StringType, env)
+	resp.Diagnostics.Append(diag...)
+	data.Env, diag = basetypes.NewListValueFrom(ctx, types.StringType, sortedKeyValues(env))
+	resp.Diagnostics.Append(diag...)
+
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 }
 
@@ -396,29 +416,36 @@ func (r *CachedImageResource) Create(ctx context.Context, req resource.CreateReq
 		data.ID = types.StringValue(digest.String())
 	}
 	// Compute the env attribute from the config map.
-	// TODO(mafredri): Convert any other relevant attributes given via schema.
+	env := make(map[string]string)
 	for key, elem := range data.ExtraEnv.Elements() {
-		data.Env = appendKnownEnvToList(data.Env, key, elem)
+		env[key] = tfValueToString(elem)
 	}
 
-	data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_CACHE_REPO", data.CacheRepo)
-	data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_URL", data.GitURL)
+	env["ENVBUILDER_CACHE_REPO"] = tfValueToString(data.CacheRepo)
+	env["ENVBUILDER_GIT_URL"] = tfValueToString(data.GitURL)
+
 	if !data.CacheTTLDays.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_CACHE_TTL_DAYS", data.CacheTTLDays)
+		env["ENVBUILDER_CACHE_TTL_DAYS"] = tfValueToString(data.CacheTTLDays)
 	}
 	if !data.GitUsername.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_USERNAME", data.GitUsername)
+		env["ENVBUILDER_GIT_USERNAME"] = tfValueToString(data.GitUsername)
 	}
 	if !data.GitPassword.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_GIT_PASSWORD", data.GitPassword)
+		env["ENVBUILDER_GIT_PASSWORD"] = tfValueToString(data.GitPassword)
 	}
 	// Default to remote build mode.
 	if data.RemoteRepoBuildMode.IsNull() {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_REMOTE_REPO_BUILD_MODE", types.BoolValue(true))
+		env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = "true"
 	} else {
-		data.Env = appendKnownEnvToList(data.Env, "ENVBUILDER_REMOTE_REPO_BUILD_MODE", data.RemoteRepoBuildMode)
+		env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = tfValueToString(data.RemoteRepoBuildMode)
 	}
 
+	var diag diag.Diagnostics
+	data.EnvMap, diag = basetypes.NewMapValueFrom(ctx, types.StringType, env)
+	resp.Diagnostics.Append(diag...)
+	data.Env, diag = basetypes.NewListValueFrom(ctx, types.StringType, sortedKeyValues(env))
+	resp.Diagnostics.Append(diag...)
+
 	// Save data into Terraform state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 }
@@ -652,19 +679,8 @@ func tfValueToString(val attr.Value) string {
 	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
-	}
-	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
-}
-
+// tfListToStringSlice converts a types.List to a []string by calling
+// tfValueToString on each element.
 func tfListToStringSlice(l types.List) []string {
 	var ss []string
 	for _, el := range l.Elements() {
@@ -692,3 +708,19 @@ func tfLogFunc(ctx context.Context) eblog.Func {
 		logFn(ctx, fmt.Sprintf(format, args...))
 	}
 }
+
+// sortedKeyValues returns the keys and values of the map in the form "key=value"
+// sorted by key in lexicographical order.
+func sortedKeyValues(m map[string]string) []string {
+	pairs := make([]string, 0, len(m))
+	var sb strings.Builder
+	for k := range m {
+		_, _ = sb.WriteString(k)
+		_, _ = sb.WriteRune('=')
+		_, _ = sb.WriteString(m[k])
+		pairs = append(pairs, sb.String())
+		sb.Reset()
+	}
+	sort.Strings(pairs)
+	return pairs
+}

internal/provider/cached_image_resource_test.go

@@ -24,12 +24,18 @@ func TestAccCachedImageResource(t *testing.T) {
 		files map[string]string
 	}{
 		{
+			// This test case is the simplest possible case: a devcontainer.json.
+			// However, it also makes sure we are able to generate a Dockerfile
+			// from the devcontainer.json.
 			name: "devcontainer only",
 			files: map[string]string{
 				".devcontainer/devcontainer.json": `{"image": "localhost:5000/test-ubuntu:latest"}`,
 			},
 		},
 		{
+			// This test case includes a Dockerfile in addition to the devcontainer.json.
+			// The Dockerfile writes the current date to a file. This is currently not checked but
+			// illustrates that a RUN instruction is cached.
 			name: "devcontainer and Dockerfile",
 			files: map[string]string{
 				".devcontainer/devcontainer.json": `{"build": { "dockerfile": "Dockerfile" }}`,
@@ -46,20 +52,19 @@ RUN date > /date.txt`,
 			resource.Test(t, resource.TestCase{
 				ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 				Steps: []resource.TestStep{
-					// Initial state: cache has not been seeded.
+					// 1) Initial state: cache has not been seeded.
 					{
 						Config:             deps.Config(t),
 						PlanOnly:           true,
 						ExpectNonEmptyPlan: true,
 					},
-					// Should detect that no cached image is present and plan to create the resource.
+					// 2) Should detect that no cached image is present and plan to create the resource.
 					{
 						Config: deps.Config(t),
 						Check: resource.ComposeAggregateTestCheckFunc(
 							// Computed values MUST be present.
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "id", uuid.Nil.String()),
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "exists", "false"),
-							resource.TestCheckResourceAttrSet("envbuilder_cached_image.test", "env.0"),
 							// Cached image should be set to the builder image.
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "image", deps.BuilderImage),
 							// Inputs should still be present.
@@ -70,17 +75,18 @@ RUN date > /date.txt`,
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"),
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_password"),
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "cache_ttl_days"),
+							// Environment variables
+							assertEnv(t, deps),
 						),
 						ExpectNonEmptyPlan: true, // TODO: check the plan.
 					},
-					// Re-running plan should have the same effect.
+					// 3) Re-running plan should have the same effect.
 					{
 						Config: deps.Config(t),
 						Check: resource.ComposeAggregateTestCheckFunc(
 							// Computed values MUST be present.
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "id", uuid.Nil.String()),
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "exists", "false"),
-							resource.TestCheckResourceAttrSet("envbuilder_cached_image.test", "env.0"),
 							// Cached image should be set to the builder image.
 							resource.TestCheckResourceAttr("envbuilder_cached_image.test", "image", deps.BuilderImage),
 							// Inputs should still be present.
@@ -91,10 +97,12 @@ RUN date > /date.txt`,
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"),
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_password"),
 							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "cache_ttl_days"),
+							// Environment variables
+							assertEnv(t, deps),
 						),
 						ExpectNonEmptyPlan: true, // TODO: check the plan.
 					},
-					// Now, seed the cache and re-run. We should now successfully create the cached image resource.
+					// 4) Now, seed the cache and re-run. We should now successfully create the cached image resource.
 					{
 						PreConfig: func() {
 							seedCache(ctx, t, deps)
@@ -114,19 +122,16 @@ RUN date > /date.txt`,
 							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\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"),
-							resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "env.4"),
+							// Environment variables
+							assertEnv(t, deps),
 						),
 					},
-					// Should produce an empty plan after apply
+					// 5) Should produce an empty plan after apply
 					{
 						Config:   deps.Config(t),
 						PlanOnly: true,
 					},
-					// Ensure idempotence in this state!
+					// 6) Ensure idempotence in this state!
 					{
 						Config:   deps.Config(t),
 						PlanOnly: true,
@@ -136,3 +141,24 @@ RUN date > /date.txt`,
 		})
 	}
 }
+
+// assertEnv is a test helper that checks the environment variables set on the
+// cached image resource based on the provided test dependencies.
+func assertEnv(t *testing.T, deps testDependencies) resource.TestCheckFunc {
+	t.Helper()
+	return resource.ComposeAggregateTestCheckFunc(
+		// Check that the environment variables are set correctly.
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.0", fmt.Sprintf("ENVBUILDER_CACHE_REPO=%s", deps.CacheRepo)),
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.1", fmt.Sprintf("ENVBUILDER_GIT_URL=%s", deps.Repo.URL)),
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.2", "ENVBUILDER_REMOTE_REPO_BUILD_MODE=true"),
+		// Check that the extra environment variables are set correctly.
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.3", "FOO=bar\nbaz"),
+		// We should not have any other environment variables set.
+		resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "env.4"),
+		// Check that the same values are set in env_map.
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.FOO", "bar\nbaz"),
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.ENVBUILDER_CACHE_REPO", deps.CacheRepo),
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.ENVBUILDER_GIT_URL", deps.Repo.URL),
+		resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.ENVBUILDER_REMOTE_REPO_BUILD_MODE", "true"),
+	)
+}