Merge pull request #97 from axiomhq/reject-map-fields-on-metrics-datasets

dataset: reject `map_fields` on `otel:metrics:v1` datasets

Author: a-khaledf

axiom/provider_test.go

@@ -747,6 +747,72 @@ func TestAccAxiomResources_dataset_map_fields(t *testing.T) {
 	})
 }
 
+func TestAccAxiomResources_dataset_map_fields_metrics_kind(t *testing.T) {
+	client, err := ax.NewClient()
+	assert.NoError(t, err)
+
+	datasetName := "new-dataset-metrics-mapfields-" + uuid.NewString()
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
+			"axiom": providerserver.NewProtocol6WithError(NewAxiomProvider()),
+		},
+		CheckDestroy: testAccCheckAxiomResourcesDestroyed(client),
+		Steps: []resource.TestStep{
+			// map-fields are not supported for metrics datasets
+			{
+				Config: `
+					provider "axiom" {
+						api_token = "` + os.Getenv("AXIOM_TOKEN") + `"
+						base_url  = "` + os.Getenv("AXIOM_URL") + `"
+					}
+
+					resource "axiom_dataset" "test" {
+						name       = "` + datasetName + `"
+						kind       = "otel:metrics:v1"
+						map_fields = ["field1"]
+					}
+				`,
+				ExpectError: regexp.MustCompile(`Error:\sInvalid\sAttribute\sCombination`),
+			},
+			// an empty list is still an explicit map-fields configuration
+			{
+				Config: `
+					provider "axiom" {
+						api_token = "` + os.Getenv("AXIOM_TOKEN") + `"
+						base_url  = "` + os.Getenv("AXIOM_URL") + `"
+					}
+
+					resource "axiom_dataset" "test" {
+						name       = "` + datasetName + `"
+						kind       = "otel:metrics:v1"
+						map_fields = []
+					}
+				`,
+				ExpectError: regexp.MustCompile(`Error:\sInvalid\sAttribute\sCombination`),
+			},
+			// omitting map-fields is fine
+			{
+				Config: `
+					provider "axiom" {
+						api_token = "` + os.Getenv("AXIOM_TOKEN") + `"
+						base_url  = "` + os.Getenv("AXIOM_URL") + `"
+					}
+
+					resource "axiom_dataset" "test" {
+						name = "` + datasetName + `"
+						kind = "otel:metrics:v1"
+					}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("axiom_dataset.test", "kind", "otel:metrics:v1"),
+					resource.TestCheckResourceAttr("axiom_dataset.test", "map_fields.#", "0"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAxiomResources_dataset_map_kind(t *testing.T) {
 	client, err := ax.NewClient()
 	assert.NoError(t, err)

axiom/resource_dataset.go

@@ -24,10 +24,14 @@ import (
 
 var validMapFieldNameRe = regexp.MustCompile("^[a-zA-Z0-9]+([a-zA-Z0-9_.-]*[a-zA-Z0-9]+)?$")
 
+// metricsDatasetKind is the dataset kind that does not support map fields.
+const metricsDatasetKind = "otel:metrics:v1"
+
 // Ensure provider defined types fully satisfy framework interfaces.
 var (
 	_ resource.Resource                = &DatasetResource{}
 	_ resource.ResourceWithImportState = &DatasetResource{}
+	_ validator.List                   = unsupportedForKindValidator{}
 )
 
 func NewDatasetResource() resource.Resource {
@@ -125,7 +129,7 @@ func (r *DatasetResource) Schema(_ context.Context, _ resource.SchemaRequest, re
 			"map_fields": schema.ListAttribute{
 				Optional:            true,
 				Computed:            true,
-				MarkdownDescription: "Map fields for the dataset",
+				MarkdownDescription: "Map fields for the dataset. Not supported for datasets of kind 'otel:metrics:v1'",
 				ElementType:         types.StringType,
 				Validators: []validator.List{
 					listvalidator.All(
@@ -136,13 +140,57 @@ func (r *DatasetResource) Schema(_ context.Context, _ resource.SchemaRequest, re
 							),
 						),
 						listvalidator.UniqueValues(),
+						unsupportedForKindValidator{kind: metricsDatasetKind},
 					),
 				},
 			},
 		},
 	}
 }
 
+// unsupportedForKindValidator rejects a list attribute that is set on a dataset of
+// an unsupported kind.
+type unsupportedForKindValidator struct {
+	kind string
+}
+
+func (v unsupportedForKindValidator) Description(_ context.Context) string {
+	return fmt.Sprintf("must not be set when kind is %q", v.kind)
+}
+
+func (v unsupportedForKindValidator) MarkdownDescription(ctx context.Context) string {
+	return v.Description(ctx)
+}
+
+func (v unsupportedForKindValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) {
+	var kind types.String
+
+	resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("kind"), &kind)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	if !attributeConflictsWithKind(kind, req.ConfigValue, v.kind) {
+		return
+	}
+
+	resp.Diagnostics.AddAttributeError(
+		req.Path,
+		"Invalid Attribute Combination",
+		fmt.Sprintf("%s is not supported for datasets of kind %q, remove it from the configuration.", req.Path, v.kind),
+	)
+}
+
+// attributeConflictsWithKind reports whether the attribute is configured on a dataset
+// of the kind that does not support it.
+func attributeConflictsWithKind(kind types.String, value types.List, unsupportedKind string) bool {
+	if kind.IsNull() || kind.IsUnknown() || kind.ValueString() != unsupportedKind {
+		return false
+	}
+
+	return !value.IsNull() && !value.IsUnknown()
+}
+
 func (r *DatasetResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) {
 	if req.ProviderData == nil {
 		return

axiom/resource_dataset_unit_test.go

@@ -3,6 +3,7 @@ package axiom
 import (
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-framework/attr"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/stretchr/testify/assert"
 
@@ -121,6 +122,48 @@ func TestSelectDefaultEdgeDeployment(t *testing.T) {
 	})
 }
 
+func TestAttributeConflictsWithKind(t *testing.T) {
+	t.Parallel()
+
+	mapFields := types.ListValueMust(types.StringType, []attr.Value{types.StringValue("field1")})
+
+	t.Run("conflicts when map fields are set on a metrics dataset", func(t *testing.T) {
+		t.Parallel()
+
+		assert.True(t, attributeConflictsWithKind(types.StringValue("otel:metrics:v1"), mapFields, metricsDatasetKind))
+	})
+
+	t.Run("conflicts when map fields are empty on a metrics dataset", func(t *testing.T) {
+		t.Parallel()
+
+		empty := types.ListValueMust(types.StringType, []attr.Value{})
+
+		assert.True(t, attributeConflictsWithKind(types.StringValue("otel:metrics:v1"), empty, metricsDatasetKind))
+	})
+
+	t.Run("does not conflict when map fields are absent on a metrics dataset", func(t *testing.T) {
+		t.Parallel()
+
+		assert.False(t, attributeConflictsWithKind(types.StringValue("otel:metrics:v1"), types.ListNull(types.StringType), metricsDatasetKind))
+		assert.False(t, attributeConflictsWithKind(types.StringValue("otel:metrics:v1"), types.ListUnknown(types.StringType), metricsDatasetKind))
+	})
+
+	t.Run("does not conflict for other kinds", func(t *testing.T) {
+		t.Parallel()
+
+		for _, kind := range []string{"axiom:events:v1", "otel:traces:v1", "otel:logs:v1"} {
+			assert.False(t, attributeConflictsWithKind(types.StringValue(kind), mapFields, metricsDatasetKind), kind)
+		}
+	})
+
+	t.Run("does not conflict when kind is not known", func(t *testing.T) {
+		t.Parallel()
+
+		assert.False(t, attributeConflictsWithKind(types.StringNull(), mapFields, metricsDatasetKind))
+		assert.False(t, attributeConflictsWithKind(types.StringUnknown(), mapFields, metricsDatasetKind))
+	})
+}
+
 func TestEdgeDeploymentValue(t *testing.T) {
 	t.Parallel()
 

docs/data-sources/dataset.md

@@ -24,7 +24,7 @@ description: |-
 - `description` (String) Dataset description
 - `edge_deployment` (String) Edge deployment for the dataset (for example, 'cloud.eu-central-1.aws')
 - `kind` (String) Dataset kind. Must be one of: 'axiom:events:v1', 'otel:metrics:v1', 'otel:traces:v1', 'otel:logs:v1'. Defaults to 'axiom:events:v1'
-- `map_fields` (List of String) Map fields for the dataset
+- `map_fields` (List of String) Map fields for the dataset. Not supported for datasets of kind 'otel:metrics:v1'
 - `name` (String) Dataset name
 - `retention_days` (Number) Retention days for the dataset
 - `use_retention_period` (Boolean) Use retention for the dataset

docs/resources/dataset.md

@@ -24,7 +24,7 @@ description: |-
 - `description` (String) Dataset description
 - `edge_deployment` (String) Edge deployment for the dataset (for example, 'cloud.eu-central-1.aws')
 - `kind` (String) Dataset kind. Must be one of: 'axiom:events:v1', 'otel:metrics:v1', 'otel:traces:v1', 'otel:logs:v1'. Defaults to 'axiom:events:v1'
-- `map_fields` (List of String) Map fields for the dataset
+- `map_fields` (List of String) Map fields for the dataset. Not supported for datasets of kind 'otel:metrics:v1'
 - `retention_days` (Number) Retention days for the dataset
 - `use_retention_period` (Boolean) Use retention for the dataset