Skip to content

Commit e8324db

Browse files
johnstcnjohnstcn
committed
fix CRUD implementation
1 parent 366bc3b commit e8324db

File tree

3 files changed

+55
-28
lines changed

3 files changed

+55
-28
lines changed

‎internal/provider/cached_image_resource.go

+13-9
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type CachedImageResource struct {
4343
client *http.Client
4444
}
4545

46-
// CachedImageResourceModel describes the data source data model.
46+
// CachedImageResourceModel describes an envbuilder cached image resource.
4747
type CachedImageResourceModel struct {
4848
// Required "inputs".
4949
BuilderImage types.String `tfsdk:"builder_image"`
@@ -85,7 +85,7 @@ func (r *CachedImageResource) Metadata(ctx context.Context, req resource.Metadat
8585
func (r *CachedImageResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
8686
resp.Schema = schema.Schema{
8787
// This description is used by the documentation generator and the language server.
88-
MarkdownDescription: "The cached image data source can be used to retrieve a cached image produced by envbuilder. Reading from this data source will clone the specified Git repository, read a Devcontainer specification or Dockerfile, and check for its presence in the provided cache repo.",
88+
MarkdownDescription: "The cached image resource can be used to retrieve a cached image produced by envbuilder. Creating this resource will clone the specified Git repository, read a Devcontainer specification or Dockerfile, and check for its presence in the provided cache repo. If any of the layers of the cached image are missing in the provided cache repo, the image will be considered as missing. A cached image in this state will be recreated until found.",
8989

9090
Attributes: map[string]schema.Attribute{
9191
// Required "inputs".
@@ -240,27 +240,31 @@ func (r *CachedImageResource) Configure(ctx context.Context, req resource.Config
240240
func (r *CachedImageResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
241241
var data CachedImageResourceModel
242242

243-
// Read Terraform configuration data into the model
243+
// Read prior state into the model
244244
resp.Diagnostics.Append(req.State.Get(ctx, &data)...)
245245
if resp.Diagnostics.HasError() {
246246
return
247247
}
248248

249-
// If the computed attribute 'image' has not been set, then there is nothing
250-
// to read!
251-
if data.Image.IsUnknown() {
249+
// If the previous state is that Image == BuilderImage, then we previously did
250+
// not find the image. We will need to run another cache probe.
251+
if data.Image.Equal(data.BuilderImage) {
252+
data.ID = types.StringValue(uuid.Nil.String())
253+
resp.State.RemoveResource(ctx)
252254
return
253255
}
254256

255-
// Attempt to fetch the image manifest.
257+
// Check the remote registry for the image we previously found.
256258
img, err := getRemoteImage(data.Image.ValueString())
257259
if err != nil {
258260
if !strings.Contains(err.Error(), "MANIFEST_UNKNOWN") {
259261
resp.Diagnostics.AddError("Error checking remote image", err.Error())
260262
return
261263
}
262264
tflog.Debug(ctx, "Remote image does not exist", map[string]any{"ref": data.Image.ValueString()})
263-
// Image does not exist
265+
// Image does not exist any longer! Remove the resource so we can re-create
266+
// it next time.
267+
resp.State.RemoveResource(ctx)
264268
return
265269
}
266270

@@ -312,7 +316,7 @@ func (r *CachedImageResource) Create(ctx context.Context, req resource.CreateReq
312316
// FIXME: there are legit errors that can crop up here.
313317
// We should add a sentinel error in Kaniko for uncached layers, and check
314318
// it here.
315-
resp.Diagnostics.AddWarning("Cached Image Not Found", fmt.Sprintf("Unable to check for cached image: %s", err.Error()))
319+
tflog.Info(ctx, "cached image not found", map[string]any{"err": err.Error()})
316320
data.Image = data.BuilderImage
317321
} else if digest, err := cachedImg.Digest(); err != nil {
318322
// There's something seriously up with this image!

‎internal/provider/cached_image_resource_test.go

+31-19
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package provider
66
import (
77
"context"
88
"fmt"
9-
"strings"
109
"testing"
1110
"time"
1211

@@ -34,7 +33,7 @@ func TestAccCachedImageDataSource(t *testing.T) {
3433
PlanOnly: true,
3534
ExpectNonEmptyPlan: true,
3635
},
37-
// Should detect that no cached image is present.
36+
// Should detect that no cached image is present and plan to create the resource.
3837
{
3938
Config: deps.Config(t),
4039
Check: resource.ComposeAggregateTestCheckFunc(
@@ -53,8 +52,30 @@ func TestAccCachedImageDataSource(t *testing.T) {
5352
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_password"),
5453
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "cache_ttl_days"),
5554
),
55+
ExpectNonEmptyPlan: true, // TODO: check the plan.
5656
},
57-
// Now, seed the cache. We should detect the cached image resource.
57+
// Re-running plan should have the same effect.
58+
{
59+
Config: deps.Config(t),
60+
Check: resource.ComposeAggregateTestCheckFunc(
61+
// Computed values MUST be present.
62+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "id", uuid.Nil.String()),
63+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "exists", "false"),
64+
resource.TestCheckResourceAttrSet("envbuilder_cached_image.test", "env.0"),
65+
// Cached image should be set to the builder image.
66+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "image", deps.BuilderImage),
67+
// Inputs should still be present.
68+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "cache_repo", deps.CacheRepo),
69+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "extra_env.FOO", "bar"),
70+
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "git_url", deps.Repo.URL),
71+
// Should be empty
72+
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_username"),
73+
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_password"),
74+
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "cache_ttl_days"),
75+
),
76+
ExpectNonEmptyPlan: true, // TODO: check the plan.
77+
},
78+
// Now, seed the cache and re-run. We should now successfully create the cached image resource.
5879
{
5980
PreConfig: func() {
6081
seedCache(ctx, t, deps)
@@ -70,24 +91,10 @@ func TestAccCachedImageDataSource(t *testing.T) {
7091
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "git_password"),
7192
resource.TestCheckNoResourceAttr("envbuilder_cached_image.test", "cache_ttl_days"),
7293
// Computed
73-
resource.TestCheckResourceAttrWith("envbuilder_cached_image.test", "id", func(value string) error {
74-
// value is enclosed in quotes
75-
value = strings.Trim(value, `"`)
76-
if !strings.HasPrefix(value, "sha256:") {
77-
return fmt.Errorf("expected image %q to have prefix %q", value, deps.CacheRepo)
78-
}
79-
return nil
80-
}),
94+
resource.TestCheckResourceAttrWith("envbuilder_cached_image.test", "id", quotedPrefix("sha256:")),
8195
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "exists", "true"),
8296
resource.TestCheckResourceAttrSet("envbuilder_cached_image.test", "image"),
83-
resource.TestCheckResourceAttrWith("envbuilder_cached_image.test", "image", func(value string) error {
84-
// value is enclosed in quotes
85-
value = strings.Trim(value, `"`)
86-
if !strings.HasPrefix(value, deps.CacheRepo) {
87-
return fmt.Errorf("expected image %q to have prefix %q", value, deps.CacheRepo)
88-
}
89-
return nil
90-
}),
97+
resource.TestCheckResourceAttrWith("envbuilder_cached_image.test", "image", quotedPrefix(deps.CacheRepo)),
9198
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.0", "FOO=\"bar\""),
9299
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.1", fmt.Sprintf("ENVBUILDER_CACHE_REPO=%q", deps.CacheRepo)),
93100
resource.TestCheckResourceAttr("envbuilder_cached_image.test", "env.2", fmt.Sprintf("ENVBUILDER_GIT_URL=%q", deps.Repo.URL)),
@@ -98,6 +105,11 @@ func TestAccCachedImageDataSource(t *testing.T) {
98105
Config: deps.Config(t),
99106
PlanOnly: true,
100107
},
108+
// Ensure idempotence in this state!
109+
{
110+
Config: deps.Config(t),
111+
PlanOnly: true,
112+
},
101113
},
102114
})
103115
}

‎internal/provider/provider_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,14 @@ func ensureImage(ctx context.Context, t testing.TB, cli *client.Client, ref stri
208208
_, err = io.ReadAll(resp)
209209
require.NoError(t, err)
210210
}
211+
212+
// quotedPrefix is a helper for asserting quoted strings.
213+
func quotedPrefix(prefix string) func(string) error {
214+
return func(val string) error {
215+
trimmed := strings.Trim(val, `"`)
216+
if !strings.HasPrefix(trimmed, prefix) {
217+
return fmt.Errorf("expected value %q to have prefix %q", trimmed, prefix)
218+
}
219+
return nil
220+
}
221+
}

0 commit comments

Comments
 (0)