tag: accept multiple tags per resource (#74)

This makes `oci_tag` accept multiple tags, so we don't need a TF
resource per tag. Plopping that many resources into TF state seems to
make it slow.

Instead we'll deprecate `tag` in favor of `tags`, which does the same
thing by applying multiple tags per resource.

After callers are migrated to `tags` we can remove the deprecated field.

---------

Signed-off-by: Jason Hall <[email protected]>

Author: imjasonh

docs/resources/tag.md

@@ -18,7 +18,11 @@ Tag an existing image by digest.
 ### Required
 
 - `digest_ref` (String) Image ref by digest to apply the tag to.
-- `tag` (String) Tag to apply to the image.
+
+### Optional
+
+- `tag` (String, Deprecated) Tag to apply to the image.
+- `tags` (List of String) Tags to apply to the image.
 
 ### Read-Only
 

internal/provider/tag_resource.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/chainguard-dev/terraform-provider-oci/pkg/validators"
@@ -10,10 +11,13 @@ import (
 	"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/listplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
+	"golang.org/x/sync/errgroup"
 )
 
 var _ resource.Resource = &TagResource{}
@@ -35,6 +39,7 @@ type TagResourceModel struct {
 
 	DigestRef types.String `tfsdk:"digest_ref"`
 	Tag       types.String `tfsdk:"tag"`
+	Tags      []string     `tfsdk:"tags"`
 }
 
 func (r *TagResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
@@ -53,11 +58,19 @@ func (r *TagResource) Schema(ctx context.Context, req resource.SchemaRequest, re
 			},
 			"tag": schema.StringAttribute{
 				MarkdownDescription: "Tag to apply to the image.",
-				Required:            true,
+				Optional:            true,
 				Validators:          []validator.String{validators.TagValidator{}},
 				PlanModifiers:       []planmodifier.String{stringplanmodifier.RequiresReplace()},
+				DeprecationMessage:  "The `tag` attribute is deprecated. Use `tags` instead.",
+			},
+			"tags": schema.ListAttribute{
+				MarkdownDescription: "Tags to apply to the image.",
+				// TODO: make this required after tag deprecation period.
+				Optional:      true,
+				ElementType:   basetypes.StringType{},
+				Validators:    []validator.List{uniqueTagsValidator{}},
+				PlanModifiers: []planmodifier.List{listplanmodifier.RequiresReplace()},
 			},
-
 			"tagged_ref": schema.StringAttribute{
 				Computed:            true,
 				MarkdownDescription: "The resulting fully-qualified image ref by digest (e.g. {repo}:tag@sha256:deadbeef).",
@@ -113,8 +126,8 @@ func (r *TagResource) Read(ctx context.Context, req resource.ReadRequest, resp *
 		return
 	}
 
-	// Don't actually tag, but check whether the digest is already tagged so we get a useful diff.
-	// If the digest is already tagged, we'll set the ID and tagged_ref to the correct output value.
+	// Don't actually tag, but check whether the digest is already tagged with all requested tags, so we get a useful diff.
+	// If the digest is already tagged with all requested tags, we'll set the ID and tagged_ref to the correct output value.
 	// Otherwise, we'll set them to empty strings so that the create will run when applied.
 
 	d, err := name.NewDigest(data.DigestRef.ValueString())
@@ -123,22 +136,38 @@ func (r *TagResource) Read(ctx context.Context, req resource.ReadRequest, resp *
 		return
 	}
 
-	t := d.Context().Tag(data.Tag.ValueString())
-	desc, err := remote.Get(t, r.popts.withContext(ctx)...)
-	if err != nil {
-		resp.Diagnostics.AddError("Tag Error", fmt.Sprintf("Error getting image: %s", err.Error()))
-		return
+	tags := []string{}
+	if data.Tag.ValueString() != "" {
+		tags = append(tags, data.Tag.ValueString())
+	} else if len(data.Tags) > 0 {
+		tags = data.Tags
+	} else {
+		resp.Diagnostics.AddError("Tag Error", "either tag or tags must be set")
+	}
+	if data.Tag.ValueString() != "" && len(data.Tags) > 0 {
+		resp.Diagnostics.AddError("Tag Error", "only one of tag or tags may be set")
 	}
+	for _, tag := range tags {
+		t := d.Context().Tag(tag)
+		desc, err := remote.Get(t, r.popts.withContext(ctx)...)
+		if err != nil {
+			// Failed to get the image by tag, so we need to create.
+			return
+		}
 
-	if desc.Digest.String() != d.DigestStr() {
-		data.Id = types.StringValue("")
-		data.TaggedRef = types.StringValue("")
-	} else {
-		id := fmt.Sprintf("%s@%s", t.Name(), desc.Digest.String())
-		data.Id = types.StringValue(id)
-		data.TaggedRef = types.StringValue(id)
+		// Some tag is wrong, so we need to create.
+		if desc.Digest.String() != d.DigestStr() {
+			data.Id = types.StringValue("")
+			data.TaggedRef = types.StringValue("")
+			break
+		}
 	}
 
+	// All tags are correct so we can set the ID and tagged_ref to the correct output value.
+	id := fmt.Sprintf("%s@%s", d.Context().Tag(tags[0]), d.DigestStr())
+	data.Id = types.StringValue(id)
+	data.TaggedRef = types.StringValue(id)
+
 	// Save updated data into Terraform state
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 }
@@ -171,21 +200,83 @@ func (r *TagResource) ImportState(ctx context.Context, req resource.ImportStateR
 }
 
 func (r *TagResource) doTag(ctx context.Context, data *TagResourceModel) (string, error) {
+	var tags []string
+	if data.Tag.ValueString() != "" {
+		tags = append(tags, data.Tag.ValueString())
+	} else if len(data.Tags) > 0 {
+		tags = data.Tags
+	} else {
+		return "", errors.New("either tag or tags must be set")
+	}
+	if data.Tag.ValueString() != "" && len(data.Tags) > 0 {
+		return "", errors.New("only one of tag or tags may be set")
+	}
+
 	d, err := name.NewDigest(data.DigestRef.ValueString())
 	if err != nil {
 		return "", fmt.Errorf("digest_ref must be a digest reference: %v", err)
 	}
-	t := d.Context().Tag(data.Tag.ValueString())
-	if err != nil {
-		return "", fmt.Errorf("error parsing tag: %v", err)
-	}
+
 	desc, err := remote.Get(d, r.popts.withContext(ctx)...)
 	if err != nil {
 		return "", fmt.Errorf("error fetching digest: %v", err)
 	}
-	if err := remote.Tag(t, desc, r.popts.withContext(ctx)...); err != nil {
-		return "", fmt.Errorf("error tagging digest: %v", err)
+
+	errg, ctx := errgroup.WithContext(ctx)
+	for _, tag := range tags {
+		tag := tag
+		errg.Go(func() error {
+			t := d.Context().Tag(tag)
+			if err != nil {
+				return fmt.Errorf("error parsing tag %q: %v", tag, err)
+			}
+			if err := remote.Tag(t, desc, r.popts.withContext(ctx)...); err != nil {
+				return fmt.Errorf("error tagging digest with %q: %v", tag, err)
+			}
+			return nil
+		})
+	}
+	if err := errg.Wait(); err != nil {
+		return "", err
+	}
+
+	t := d.Context().Tag(tags[0])
+	if err != nil {
+		return "", fmt.Errorf("error parsing tag: %v", err)
 	}
-	digest := fmt.Sprintf("%s@%s", t.Name(), desc.Digest.String())
+	digest := fmt.Sprintf("%s@%s", t.Name(), d.DigestStr())
 	return digest, nil
 }
+
+type uniqueTagsValidator struct{}
+
+var _ validator.List = uniqueTagsValidator{}
+
+func (v uniqueTagsValidator) Description(context.Context) string {
+	return `value must be valid OCI tag elements (e.g., "latest", "v1.2.3")`
+}
+func (v uniqueTagsValidator) MarkdownDescription(ctx context.Context) string {
+	return v.Description(ctx)
+}
+
+func (v uniqueTagsValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) {
+	if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() {
+		return
+	}
+	var tags []string
+	if diag := req.ConfigValue.ElementsAs(ctx, &tags, false); diag.HasError() {
+		resp.Diagnostics.Append(diag...)
+		return
+	}
+
+	seen := map[string]bool{}
+	for _, t := range tags {
+		if seen[t] {
+			resp.Diagnostics.AddWarning("Duplicate tag", fmt.Sprintf("duplicate tag %q", t))
+		}
+		seen[t] = true
+		if _, err := name.NewTag("example.com:" + t); err != nil {
+			resp.Diagnostics.AddError("Invalid OCI tag name", fmt.Sprintf("parsing tag %q: %v", t, err))
+		}
+	}
+}

internal/provider/tag_resource_test.go

@@ -11,7 +11,7 @@ import (
 )
 
 func TestAccTagResource(t *testing.T) {
-	repo, cleanup := ocitesting.SetupRepository(t, "test")
+	repo, cleanup := ocitesting.SetupRepository(t, "repo")
 	defer cleanup()
 
 	// Push an image to the local registry.
@@ -30,7 +30,7 @@ func TestAccTagResource(t *testing.T) {
 	}
 	dig1 := ref1.Context().Digest(d1.String())
 
-	// Push an image to the local registry.
+	// Push another image to the local registry.
 	ref2 := repo.Tag("2")
 	t.Logf("Using ref2: %s", ref2)
 	img2, err := random.Image(1024, 1)
@@ -46,36 +46,108 @@ func TestAccTagResource(t *testing.T) {
 	}
 	dig2 := ref2.Context().Digest(d2.String())
 
-	want1 := fmt.Sprintf("%s:test@%s", repo, d2)
-	want2 := fmt.Sprintf("%s:test2@%s", repo, d2)
-
+	// Create the resource tagging dig1 three tags.
+	want1 := fmt.Sprintf("%s:foo@%s", repo, d1)
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
-			// Create and Read testing
 			{
 				Config: fmt.Sprintf(`resource "oci_tag" "test" {
 				  digest_ref = %q
-				  tag        = "test"
+				  tags       = ["foo", "bar", "baz"]
 				}`, dig1),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("oci_tag.test", "tagged_ref", want1),
 					resource.TestCheckResourceAttr("oci_tag.test", "id", want1),
 				),
 			},
-			// Update and Read testing
+		},
+	})
+
+	// The digest should be tagged with all three tags.
+	for _, want := range []string{"foo", "bar", "baz"} {
+		desc, err := remote.Get(repo.Tag(want))
+		if err != nil {
+			t.Errorf("failed to get image with tag %q: %v", want, err)
+		}
+		if desc.Digest != d1 {
+			t.Errorf("image with tag %q has wrong digest: got %s, want %s", want, desc.Digest, d1)
+		}
+	}
+
+	// Point the tags to another digest.
+	want2 := fmt.Sprintf("%s:foo@%s", repo, d2)
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
 			{
 				Config: fmt.Sprintf(`resource "oci_tag" "test" {
-					digest_ref = %q
-					tag        = "test2"
-				  }`, dig2),
+				  digest_ref = %q
+				  tags       = ["foo", "bar", "baz"]
+				}`, dig2),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("oci_tag.test", "tagged_ref", want2),
 					resource.TestCheckResourceAttr("oci_tag.test", "id", want2),
 				),
 			},
-			// Delete testing automatically occurs in TestCase
 		},
 	})
+
+	// The second digest should be tagged with all three tags.
+	for _, want := range []string{"foo", "bar", "baz"} {
+		desc, err := remote.Get(repo.Tag(want))
+		if err != nil {
+			t.Errorf("failed to get image with tag %q: %v", want, err)
+		}
+		if desc.Digest != d2 {
+			t.Errorf("image with tag %q has wrong digest: got %s, want %s", want, desc.Digest, d2)
+		}
+	}
+
+	// Add a fourth tag.
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`resource "oci_tag" "test" {
+					  digest_ref = %q
+					  tags       = ["foo", "bar", "baz", "qux"]
+					}`, dig2),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("oci_tag.test", "tagged_ref", want2),
+					resource.TestCheckResourceAttr("oci_tag.test", "id", want2),
+				),
+			},
+		},
+	})
+
+	// The second digest should be tagged with all three tags.
+	for _, want := range []string{"foo", "bar", "baz", "qux"} {
+		desc, err := remote.Get(repo.Tag(want))
+		if err != nil {
+			t.Errorf("failed to get image with tag %q: %v", want, err)
+		}
+		if desc.Digest != d2 {
+			t.Errorf("image with tag %q has wrong digest: got %s, want %s", want, desc.Digest, d2)
+		}
+	}
+
+	// Tag the digest with the same tag multiple times, which should be allowed but warn.
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{{
+			Config: fmt.Sprintf(`resource "oci_tag" "test" {
+					  digest_ref = %q
+					  tags       = ["foo", "foo", "foo", "bar", "bar", "bar"]
+					}`, dig2),
+			Check: resource.ComposeAggregateTestCheckFunc(
+				resource.TestCheckResourceAttr("oci_tag.test", "tagged_ref", want2),
+				resource.TestCheckResourceAttr("oci_tag.test", "id", want2),
+			),
+		}},
+	})
 }

pkg/validators/tag_validator.go

@@ -10,7 +10,7 @@ import (
 // TagValidator is a string validator that checks that the string is valid OCI reference by digest.
 type TagValidator struct{}
 
-var _ validator.String = DigestValidator{}
+var _ validator.String = TagValidator{}
 
 func (v TagValidator) Description(context.Context) string {
 	return `value must be a valid OCI tag element (e.g., "latest", "v1.2.3")`