From c6bb4c412b7f99362dfcf7d9c6fe54a5ca17f8ca Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 15 Aug 2024 22:06:03 +0100 Subject: [PATCH 1/2] feat(internal/provider): add env_map to cached_image_resource --- docs/resources/cached_image.md | 3 +- internal/provider/cached_image_resource.go | 96 ++++++++++++------- .../provider/cached_image_resource_test.go | 52 +++++++--- 3 files changed, 105 insertions(+), 46 deletions(-) diff --git a/docs/resources/cached_image.md b/docs/resources/cached_image.md index 8498720..e29c7d1 100644 --- a/docs/resources/cached_image.md +++ b/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 stringss 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. diff --git a/internal/provider/cached_image_resource.go b/internal/provider/cached_image_resource.go index 6505736..2e5a477 100644 --- a/internal/provider/cached_image_resource.go +++ b/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,9 +230,8 @@ 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 stringss of `key=value`. May contain secrets.", ElementType: types.StringType, Computed: true, Sensitive: true, @@ -236,6 +239,15 @@ func (r *CachedImageResource) Schema(ctx context.Context, req resource.SchemaReq 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 +} diff --git a/internal/provider/cached_image_resource_test.go b/internal/provider/cached_image_resource_test.go index 5eeddfa..929c7b0 100644 --- a/internal/provider/cached_image_resource_test.go +++ b/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"), + ) +} From 98e16cb7dd9f727d5b8a42edc68c8624bf1c009f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 15 Aug 2024 22:16:45 +0100 Subject: [PATCH 2/2] fixup! feat(internal/provider): add env_map to cached_image_resource --- docs/resources/cached_image.md | 2 +- internal/provider/cached_image_resource.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/resources/cached_image.md b/docs/resources/cached_image.md index e29c7d1..a7f4a38 100644 --- a/docs/resources/cached_image.md +++ b/docs/resources/cached_image.md @@ -48,7 +48,7 @@ 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 in the form of a list of stringss of `key=value`. 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. diff --git a/internal/provider/cached_image_resource.go b/internal/provider/cached_image_resource.go index 2e5a477..4ea28b8 100644 --- a/internal/provider/cached_image_resource.go +++ b/internal/provider/cached_image_resource.go @@ -231,7 +231,7 @@ func (r *CachedImageResource) Schema(ctx context.Context, req resource.SchemaReq // Computed "outputs". "env": schema.ListAttribute{ - MarkdownDescription: "Computed envbuilder configuration to be set for the container in the form of a list of stringss of `key=value`. 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,