Skip to content

fix(internal/provider): set all supported envbuilder options #38

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.

Sign up for GitHub

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 2 commits into from
Aug 16, 2024
Merged

fix(internal/provider): set all supported envbuilder options #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ff5764
fix(internal/provider): set all supported envbuilder options
johnstcn Aug 16, 2024
6bec290
do not allow overriding git_url and cache_repo
johnstcn Aug 16, 2024
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
193 changes: 134 additions & 59 deletions internal/provider/cached_image_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"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 @@ -160,7 +161,6 @@ func (r *CachedImageResource) Schema(ctx context.Context, req resource.SchemaReq
MarkdownDescription: "(Envbuilder option) Terminates upon a build failure. This is handy when preferring the FALLBACK_IMAGE in cases where no devcontainer.json or image is provided. However, it ensures that the container stops if the build process encounters an error.",
Optional: true,
},
// TODO(mafredri): Map vs List? Support both?
"extra_env": schema.MapAttribute{
MarkdownDescription: "Extra environment variables to set for the container. This may include envbuilder options.",
ElementType: types.StringType,
Expand Down Expand Up @@ -293,6 +293,135 @@ func (r *CachedImageResource) Configure(ctx context.Context, req resource.Config
r.client = client
}

// setComputedEnv sets data.Env and data.EnvMap based on the values of the
// other fields in the model.
func (data *CachedImageResourceModel) setComputedEnv(ctx context.Context) diag.Diagnostics {
env := make(map[string]string)

env["ENVBUILDER_CACHE_REPO"] = tfValueToString(data.CacheRepo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting edge-case, what if extra env overrides cache repo? We use data.CacheRepo directly elsewhere, but it may be different in computed env.

We should probably always prefer the values from computed env when running the cache probe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably always prefer the values from computed env when running the cache probe?

I don't agree. The provider should use the arguments it is supplied. We add a warning diag if you override existing environment variables set, which should be enough of an indication.

Alternatively, I could add a check for this and make it a diag.Error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to block it with an error, so I'm just going to not allow overriding these but warn if they are set.

env["ENVBUILDER_GIT_URL"] = tfValueToString(data.GitURL)

if !data.BaseImageCacheDir.IsNull() {
env["ENVBUILDER_BASE_IMAGE_CACHE_DIR"] = tfValueToString(data.BaseImageCacheDir)
}

if !data.BuildContextPath.IsNull() {
env["ENVBUILDER_BUILD_CONTEXT_PATH"] = tfValueToString(data.BuildContextPath)
}

if !data.CacheTTLDays.IsNull() {
env["ENVBUILDER_CACHE_TTL_DAYS"] = tfValueToString(data.CacheTTLDays)
}

if !data.DevcontainerDir.IsNull() {
env["ENVBUILDER_DEVCONTAINER_DIR"] = tfValueToString(data.DevcontainerDir)
}

if !data.DevcontainerJSONPath.IsNull() {
env["ENVBUILDER_DEVCONTAINER_JSON_PATH"] = tfValueToString(data.DevcontainerJSONPath)
}

if !data.DockerfilePath.IsNull() {
env["ENVBUILDER_DOCKERFILE_PATH"] = tfValueToString(data.DockerfilePath)
}

if !data.DockerConfigBase64.IsNull() {
env["ENVBUILDER_DOCKER_CONFIG_BASE64"] = tfValueToString(data.DockerConfigBase64)
}

if !data.ExitOnBuildFailure.IsNull() {
env["ENVBUILDER_EXIT_ON_BUILD_FAILURE"] = tfValueToString(data.ExitOnBuildFailure)
}

if !data.FallbackImage.IsNull() {
env["ENVBUILDER_FALLBACK_IMAGE"] = tfValueToString(data.FallbackImage)
}

if !data.GitCloneDepth.IsNull() {
env["ENVBUILDER_GIT_CLONE_DEPTH"] = tfValueToString(data.GitCloneDepth)
}

if !data.GitCloneSingleBranch.IsNull() {
env["ENVBUILDER_GIT_CLONE_SINGLE_BRANCH"] = tfValueToString(data.GitCloneSingleBranch)
}

if !data.GitHTTPProxyURL.IsNull() {
env["ENVBUILDER_GIT_HTTP_PROXY_URL"] = tfValueToString(data.GitHTTPProxyURL)
}

if !data.GitSSHPrivateKeyPath.IsNull() {
env["ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH"] = tfValueToString(data.GitSSHPrivateKeyPath)
}

if !data.GitUsername.IsNull() {
env["ENVBUILDER_GIT_USERNAME"] = tfValueToString(data.GitUsername)
}

if !data.GitPassword.IsNull() {
env["ENVBUILDER_GIT_PASSWORD"] = tfValueToString(data.GitPassword)
}

if !data.IgnorePaths.IsNull() {
env["ENVBUILDER_IGNORE_PATHS"] = strings.Join(tfListToStringSlice(data.IgnorePaths), ",")
}

if !data.Insecure.IsNull() {
env["ENVBUILDER_INSECURE"] = tfValueToString(data.Insecure)
}

// Default to remote build mode.
if data.RemoteRepoBuildMode.IsNull() {
env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = "true"
} else {
env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = tfValueToString(data.RemoteRepoBuildMode)
}

if !data.SSLCertBase64.IsNull() {
env["ENVBUILDER_SSL_CERT_BASE64"] = tfValueToString(data.SSLCertBase64)
}

if !data.Verbose.IsNull() {
env["ENVBUILDER_VERBOSE"] = tfValueToString(data.Verbose)
}

if !data.WorkspaceFolder.IsNull() {
env["ENVBUILDER_WORKSPACE_FOLDER"] = tfValueToString(data.WorkspaceFolder)
}

// Do ExtraEnv last so that it can override any other values.
// With one exception: ENVBUILDER_CACHE_REPO and ENVBUILDER_GIT_URL are required and should not be overridden.
// Other values set by the provider may be overridden, but will generate a warning.
var diag, ds diag.Diagnostics
if !data.ExtraEnv.IsNull() {
for key, elem := range data.ExtraEnv.Elements() {
switch key {
// These are required and should not be overridden.
case "ENVBUILDER_CACHE_REPO", "ENVBUILDER_GIT_URL":
diag.AddAttributeWarning(path.Root("extra_env"),
"Cannot override required environment variable",
fmt.Sprintf("The key %q in extra_env cannot be overridden.", key),
)
default:
if _, ok := env[key]; ok {
// This is a warning because it's possible that the user wants to override
// a value set by the provider.
diag.AddAttributeWarning(path.Root("extra_env"),
"Overriding provider environment variable",
fmt.Sprintf("The key %q in extra_env overrides an environment variable set by the provider.", key),
)
}
env[key] = tfValueToString(elem)
}
}
}

data.EnvMap, ds = basetypes.NewMapValueFrom(ctx, types.StringType, env)
diag = append(diag, ds...)
data.Env, ds = basetypes.NewListValueFrom(ctx, types.StringType, sortedKeyValues(env))
diag = append(diag, ds...)
return diag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving all of this into one place! ❤️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was driving me up the wall 🙈

}

func (r *CachedImageResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
var data CachedImageResourceModel

Expand Down Expand Up @@ -350,35 +479,7 @@ 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() {
env[key] = tfValueToString(elem)
}

env["ENVBUILDER_CACHE_REPO"] = tfValueToString(data.CacheRepo)
env["ENVBUILDER_GIT_URL"] = tfValueToString(data.GitURL)

if !data.CacheTTLDays.IsNull() {
env["ENVBUILDER_CACHE_TTL_DAYS"] = tfValueToString(data.CacheTTLDays)
}
if !data.GitUsername.IsNull() {
env["ENVBUILDER_GIT_USERNAME"] = tfValueToString(data.GitUsername)
}
if !data.GitPassword.IsNull() {
env["ENVBUILDER_GIT_PASSWORD"] = tfValueToString(data.GitPassword)
}
// Default to remote build mode.
if data.RemoteRepoBuildMode.IsNull() {
env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = "true"
} else {
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(data.setComputedEnv(ctx)...)

resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
}
Expand Down Expand Up @@ -415,36 +516,9 @@ func (r *CachedImageResource) Create(ctx context.Context, req resource.CreateReq
data.Image = types.StringValue(fmt.Sprintf("%s@%s", data.CacheRepo.ValueString(), digest))
data.ID = types.StringValue(digest.String())
}
// Compute the env attribute from the config map.
env := make(map[string]string)
for key, elem := range data.ExtraEnv.Elements() {
env[key] = tfValueToString(elem)
}

env["ENVBUILDER_CACHE_REPO"] = tfValueToString(data.CacheRepo)
env["ENVBUILDER_GIT_URL"] = tfValueToString(data.GitURL)

if !data.CacheTTLDays.IsNull() {
env["ENVBUILDER_CACHE_TTL_DAYS"] = tfValueToString(data.CacheTTLDays)
}
if !data.GitUsername.IsNull() {
env["ENVBUILDER_GIT_USERNAME"] = tfValueToString(data.GitUsername)
}
if !data.GitPassword.IsNull() {
env["ENVBUILDER_GIT_PASSWORD"] = tfValueToString(data.GitPassword)
}
// Default to remote build mode.
if data.RemoteRepoBuildMode.IsNull() {
env["ENVBUILDER_REMOTE_REPO_BUILD_MODE"] = "true"
} else {
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...)
// Set the expected environment variables.
resp.Diagnostics.Append(data.setComputedEnv(ctx)...)

// Save data into Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
Expand Down Expand Up @@ -556,6 +630,7 @@ func (r *CachedImageResource) runCacheProbe(ctx context.Context, data CachedImag
Insecure: data.Insecure.ValueBool(), // might have internal CAs?
IgnorePaths: tfListToStringSlice(data.IgnorePaths), // may need to be specified?
// The below options are not relevant and are set to their zero value explicitly.
// They must be set by extra_env.
CoderAgentSubsystem: nil,
CoderAgentToken: "",
CoderAgentURL: "",
Expand Down
17 changes: 12 additions & 5 deletions internal/provider/cached_image_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ RUN date > /date.txt`,
//nolint: paralleltest
deps := setup(ctx, t, tc.files)
deps.ExtraEnv["FOO"] = testEnvValue
deps.ExtraEnv["ENVBUILDER_GIT_URL"] = "https://not.the.real.git/url"
deps.ExtraEnv["ENVBUILDER_CACHE_REPO"] = "not-the-real-cache-repo"

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Expand Down Expand Up @@ -149,16 +151,21 @@ func assertEnv(t *testing.T, deps testDependencies) resource.TestCheckFunc {
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"),
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.1", fmt.Sprintf("ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH=%s", deps.Repo.Key)),
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.TestCheckResourceAttr("envbuilder_cached_image.test", "env.4", "ENVBUILDER_VERBOSE=true"),
// Check that the extra environment variables are set correctly.
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.3", "FOO=bar\nbaz"),
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.5", "FOO=bar\nbaz"),
// We should not have any other environment variables set.
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "env.4"),
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "env.6"),

// 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_SSH_PRIVATE_KEY_PATH", deps.Repo.Key),
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"),
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.ENVBUILDER_VERBOSE", "true"),
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env_map.FOO", "bar\nbaz"),
)
}