diff --git a/CHANGELOG.md b/CHANGELOG.md index 4511af790d..7be8590629 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ Main (unreleased) - The `mimir.rules.kubernetes` component now supports adding extra label matchers to all queries discovered via `PrometheusRule` CRDs. (@thampiotr) +- `prometheus.exporter.snmp`: The names of `target` and `walk_param` blocks can now be set + via an argument instead of block label. The block label syntax for + the `target` and `walk_param` blocks will be removed in Alloy 2.0. (@ptodev) + ### Bugfixes - Update windows_exporter from v0.27.2 vo v0.27.3: (@jkroepke) diff --git a/docs/sources/reference/components/prometheus/prometheus.exporter.snmp.md b/docs/sources/reference/components/prometheus/prometheus.exporter.snmp.md index ed21bd0423..40d031f80b 100644 --- a/docs/sources/reference/components/prometheus/prometheus.exporter.snmp.md +++ b/docs/sources/reference/components/prometheus/prometheus.exporter.snmp.md @@ -21,7 +21,8 @@ The `prometheus.exporter.snmp` component embeds prometheus.exporter.snmp "LABEL" { config_file = SNMP_CONFIG_FILE_PATH - target "TARGET_NAME" { + target { + name = TARGET_NAME address = TARGET_ADDRESS } } @@ -48,7 +49,7 @@ Omitted fields take their default values. | `targets` | `list(map(string))` | SNMP targets. | | no | The `config_file` argument points to a YAML file defining which snmp_exporter modules to use. -Refer to [snmp_exporter](https://github.com/prometheus/snmp_exporter#generating-configuration) for details on how to generate a configuration file. +Refer to [snmp_exporter][snmp_exporter-cfg] for details on how to generate a configuration file. The `config` argument must be a YAML document as string defining which SNMP modules and auths to use. `config` is typically loaded by using the exports of another component. For example, @@ -65,6 +66,8 @@ The following labels can be set to a target: * `auth`: The SNMP authentication profile to use. * `walk_params`: The config to use for this target. +[snmp_exporter-cfg]: https://github.com/prometheus/snmp_exporter#generating-configuration + ## Blocks The following blocks are supported inside the definition of @@ -81,16 +84,26 @@ The following blocks are supported inside the definition of ### target block The `target` block defines an individual SNMP target. -The `target` block may be specified multiple times to define multiple targets. The label of the block is required and will be used in the target's `job` label. +The `target` block may be specified multiple times to define multiple targets. | Name | Type | Description | Default | Required | | -------------- | -------- | --------------------------------------------------------------------- | ------- | -------- | +| `name` | `string` | The name of the target. | | yes | | `address` | `string` | The address of SNMP device. | | yes | | `module` | `string` | SNMP module to use for polling. | `""` | no | | `auth` | `string` | SNMP authentication profile to use. | `""` | no | | `walk_params` | `string` | Config to use for this target. | `""` | no | | `snmp_context` | `string` | Override the `context_name` parameter in the SNMP configuration file. | `""` | no | +The value of the `name` argument will be set as the value of the target's `job` label. + +{{< admonition type="note" >}} +Instead of a `name` argument, previous versions of Alloy used to use a block label. +See the [Deprecated features](#deprecated-features) section for more information. +{{< /admonition >}} + +If `name` is not set, the target's `job` label will be set to the label of the `target` block. + ### walk_param block The `walk_param` block defines an individual SNMP connection profile that can be used to override default SNMP settings. @@ -98,11 +111,16 @@ The `walk_param` block may be specified multiple times to define multiple SNMP c | Name | Type | Description | Default | Required | | ----------------- | ---------- | --------------------------------------------- | ------- | -------- | -| `name` | `string` | Name of the module to override. | | no | +| `name` | `string` | Name of the module to override. | | yes | | `max_repetitions` | `int` | How many objects to request with GET/GETBULK. | `25` | no | | `retries` | `int` | How many times to retry a failed request. | `3` | no | | `timeout` | `duration` | Timeout for each individual SNMP request. | | no | +{{< admonition type="note" >}} +Instead of a `name` argument, previous versions of Alloy used to use a block label. +See the [Deprecated features](#deprecated-features) section for more information. +{{< /admonition >}} + ## Exported fields {{< docs/shared lookup="reference/components/exporter-component-exports.md" source="alloy" version="" >}} @@ -123,6 +141,46 @@ debug information. `prometheus.exporter.snmp` does not expose any component-specific debug metrics. +## Deprecated features + +In previous versions of Alloy, the names `target` blocks and `walk_param` blocks were specified via a block label: + +```alloy +target "example_name_of_target" { + ... +} + +walk_param "example_name_of_walk_param" { + ... +} +``` + +Users are now advised to switch to using a `name` attribute instead: + +```alloy +target { + name = "example_name_of_target" + ... +} + +walk_param { + name = "example_name_of_walk_param" + ... +} +``` + +Block labels for `target` and `walk_param` are deprecated and will be removed in version 2.0 of Alloy. +In Alloy version 1, it will remain possible to use a block label instead of the `name` attribute. +However, the `name` attribute is documented as "required" to encourage users to use it and to remove confusion. + +The benefits of using a `name` attribute include: +* Not being subject to restrictions on [Alloy Syntax Identifiers][syntax-identifiers]. +* It is possible to get the value of a `name` argument from a component such as [local.file][] or [remote.http][]. + +[syntax-identifiers]: ../../../../get-started/configuration-syntax/syntax/#identifiers +[local.file]: ../../local.file +[remote.http]: ../../remote.http + ## Example This example uses a [`prometheus.scrape` component][scrape] to collect metrics @@ -132,23 +190,27 @@ from `prometheus.exporter.snmp`: prometheus.exporter.snmp "example" { config_file = "snmp_modules.yml" - target "network_switch_1" { + target { + name = "network_switch_1" address = "192.168.1.2" module = "if_mib" walk_params = "public" } - target "network_router_2" { + target { + name = "network_router_2" address = "192.168.1.3" module = "mikrotik" walk_params = "private" } - walk_param "private" { + walk_param { + name = "private" retries = "2" } - walk_param "public" { + walk_param { + name = "public" retries = "2" } } @@ -171,23 +233,27 @@ local.file "snmp_config" { prometheus.exporter.snmp "example" { config = local.file.snmp_config.content - target "network_switch_1" { + target { + name = "network_switch_1" address = "192.168.1.2" module = "if_mib" walk_params = "public" } - target "network_router_2" { + target { + name = "network_router_2" address = "192.168.1.3" module = "mikrotik" walk_params = "private" } - walk_param "private" { + walk_param { + name = "private" retries = "2" } - walk_param "public" { + walk_param { + name = "public" retries = "2" } } @@ -236,11 +302,13 @@ prometheus.exporter.snmp "example" { }, ] - walk_param "private" { + walk_param { + name = "private" retries = "2" } - walk_param "public" { + walk_param { + name = "public" retries = "2" } } @@ -264,11 +332,13 @@ prometheus.exporter.snmp "example" { targets = encoding.from_yaml(local.file.targets.content) - walk_param "private" { + walk_param { + name = "private" retries = "2" } - walk_param "public" { + walk_param { + name = "public" retries = "2" } } diff --git a/internal/cmd/alloylint/internal/syntaxtags/syntaxtags.go b/internal/cmd/alloylint/internal/syntaxtags/syntaxtags.go index 47ff1cb660..2a515bcbd5 100644 --- a/internal/cmd/alloylint/internal/syntaxtags/syntaxtags.go +++ b/internal/cmd/alloylint/internal/syntaxtags/syntaxtags.go @@ -279,7 +279,7 @@ func lintSyntaxTag(ty *types.Var, tag string) (diagnostics []string) { } } - case "label": + case "label", "label,optional": if name != "" { diagnostics = append(diagnostics, "label field must have an empty value for name") } diff --git a/internal/component/prometheus/exporter/snmp/snmp.go b/internal/component/prometheus/exporter/snmp/snmp.go index f3eed3ec1a..fcf8215326 100644 --- a/internal/component/prometheus/exporter/snmp/snmp.go +++ b/internal/component/prometheus/exporter/snmp/snmp.go @@ -48,7 +48,7 @@ func buildSNMPTargets(baseTarget discovery.Target, args component.Arguments) []d target[k] = v } - target["job"] = target["job"] + "/" + tgt.Name + target["job"] = target["job"] + "/" + tgt.Name() target["__param_target"] = tgt.Target if tgt.Module != "" { target["__param_module"] = tgt.Module @@ -71,7 +71,9 @@ func buildSNMPTargets(baseTarget discovery.Target, args component.Arguments) []d // SNMPTarget defines a target to be used by the exporter. type SNMPTarget struct { - Name string `alloy:",label"` + //TODO: For Alloy 2.0: Remove NameLbl and make NameArg mandatory. + NameLbl string `alloy:",label,optional"` + NameArg string `alloy:"name,attr,optional"` Target string `alloy:"address,attr"` Module string `alloy:"module,attr,optional"` Auth string `alloy:"auth,attr,optional"` @@ -86,7 +88,7 @@ func (t TargetBlock) Convert() []snmp_exporter.SNMPTarget { targets := make([]snmp_exporter.SNMPTarget, 0, len(t)) for _, target := range t { targets = append(targets, snmp_exporter.SNMPTarget{ - Name: target.Name, + Name: target.Name(), Target: target.Target, Module: target.Module, Auth: target.Auth, @@ -97,8 +99,25 @@ func (t TargetBlock) Convert() []snmp_exporter.SNMPTarget { return targets } +// Name() assumes that either NameLbl or NameArg is set. +// It also that both NameLbl and NameArg are not set at the same time, +// as this is checked during unmarshalling. +func (t *SNMPTarget) Name() string { + if t == nil { + return "" + } + + if len(t.NameArg) > 0 { + return t.NameArg + } + + return t.NameLbl +} + type WalkParam struct { - Name string `alloy:",label"` + //TODO: For Alloy 2.0: Remove NameLbl and make NameArg mandatory. + NameLbl string `alloy:",label,optional"` + NameArg string `alloy:"name,attr,optional"` MaxRepetitions uint32 `alloy:"max_repetitions,attr,optional"` Retries int `alloy:"retries,attr,optional"` Timeout time.Duration `alloy:"timeout,attr,optional"` @@ -111,7 +130,7 @@ type WalkParams []WalkParam func (w WalkParams) Convert() map[string]snmp_config.WalkParams { walkParams := make(map[string]snmp_config.WalkParams) for _, walkParam := range w { - walkParams[walkParam.Name] = snmp_config.WalkParams{ + walkParams[walkParam.Name()] = snmp_config.WalkParams{ MaxRepetitions: walkParam.MaxRepetitions, Retries: &walkParam.Retries, Timeout: walkParam.Timeout, @@ -121,6 +140,21 @@ func (w WalkParams) Convert() map[string]snmp_config.WalkParams { return walkParams } +// Name() assumes that either NameLbl or NameArg is set. +// It also that both NameLbl and NameArg are not set at the same time, +// as this is checked during unmarshalling. +func (p *WalkParam) Name() string { + if p == nil { + return "" + } + + if len(p.NameArg) > 0 { + return p.NameArg + } + + return p.NameLbl +} + type Arguments struct { ConfigFile string `alloy:"config_file,attr,optional"` Config alloytypes.OptionalSecret `alloy:"config,attr,optional"` @@ -155,7 +189,7 @@ func (t TargetsList) convert() []SNMPTarget { for _, target := range t { address, _ := getAddress(target) targets = append(targets, SNMPTarget{ - Name: target["name"], + NameArg: target["name"], Target: address, Module: target["module"], Auth: target["auth"], @@ -181,6 +215,26 @@ func (a *Arguments) UnmarshalAlloy(f func(interface{}) error) error { return fmt.Errorf("the block `target` and the attribute `targets` are mutually exclusive") } + for _, walkParam := range a.WalkParams { + if walkParam.NameArg != "" && walkParam.NameLbl != "" { + return errors.New("the label of a `walk_param` block and the `name` argument are mutually exclusive") + } + + if walkParam.NameArg == "" && walkParam.NameLbl == "" { + return errors.New("either the label of a `walk_param` block or the `name` argument must be set") + } + } + + for _, target := range a.Targets { + if target.NameArg != "" && target.NameLbl != "" { + return errors.New("the label of a `target` block and the `name` argument are mutually exclusive") + } + + if target.NameArg == "" && target.NameLbl == "" { + return errors.New("either the label of a `target` block or the `name` argument must be set") + } + } + for _, target := range a.TargetsList { if _, hasName := target["name"]; !hasName { return fmt.Errorf("all targets must have a `name`") diff --git a/internal/component/prometheus/exporter/snmp/snmp_test.go b/internal/component/prometheus/exporter/snmp/snmp_test.go index b79e8a3412..2691dc7992 100644 --- a/internal/component/prometheus/exporter/snmp/snmp_test.go +++ b/internal/component/prometheus/exporter/snmp/snmp_test.go @@ -23,7 +23,8 @@ func TestUnmarshalAlloy(t *testing.T) { auth = "public_v2" snmp_context = "testcontext" } - target "network_router_2" { + target { + name = "network-router-2" address = "192.168.1.3" module = "mikrotik" walk_params = "private" @@ -31,9 +32,10 @@ func TestUnmarshalAlloy(t *testing.T) { walk_param "private" { retries = 1 } - walk_param "public" { + walk_param { + name = "public" retries = 2 - } + } ` var args Arguments err := syntax.Unmarshal([]byte(alloyCfg), &args) @@ -41,14 +43,14 @@ func TestUnmarshalAlloy(t *testing.T) { require.Equal(t, "modules.yml", args.ConfigFile) require.Equal(t, 2, len(args.Targets)) - require.Contains(t, "network_switch_1", args.Targets[0].Name) + require.Contains(t, "network_switch_1", args.Targets[0].Name()) require.Contains(t, "192.168.1.2", args.Targets[0].Target) require.Contains(t, "if_mib", args.Targets[0].Module) require.Contains(t, "public", args.Targets[0].WalkParams) require.Contains(t, "public_v2", args.Targets[0].Auth) require.Contains(t, "testcontext", args.Targets[0].SNMPContext) - require.Contains(t, "network_router_2", args.Targets[1].Name) + require.Contains(t, "network-router-2", args.Targets[1].Name()) require.Contains(t, "192.168.1.3", args.Targets[1].Target) require.Contains(t, "mikrotik", args.Targets[1].Module) require.Contains(t, "private", args.Targets[1].WalkParams) @@ -56,9 +58,9 @@ func TestUnmarshalAlloy(t *testing.T) { require.Equal(t, 2, len(args.WalkParams)) - require.Contains(t, "private", args.WalkParams[0].Name) + require.Contains(t, "private", args.WalkParams[0].Name()) require.Equal(t, 1, args.WalkParams[0].Retries) - require.Contains(t, "public", args.WalkParams[1].Name) + require.Contains(t, "public", args.WalkParams[1].Name()) require.Equal(t, 2, args.WalkParams[1].Retries) } @@ -107,17 +109,17 @@ func TestUnmarshalAlloyTargets(t *testing.T) { require.Equal(t, 2, len(args.WalkParams)) - require.Contains(t, "private", args.WalkParams[0].Name) + require.Contains(t, "private", args.WalkParams[0].Name()) require.Equal(t, 1, args.WalkParams[0].Retries) - require.Contains(t, "public", args.WalkParams[1].Name) + require.Contains(t, "public", args.WalkParams[1].Name()) require.Equal(t, 2, args.WalkParams[1].Retries) } func TestConvertConfig(t *testing.T) { args := Arguments{ ConfigFile: "modules.yml", - Targets: TargetBlock{{Name: "network_switch_1", Target: "192.168.1.2", Module: "if_mib"}}, - WalkParams: WalkParams{{Name: "public", Retries: 2}}, + Targets: TargetBlock{{NameLbl: "network_switch_1", Target: "192.168.1.2", Module: "if_mib"}}, + WalkParams: WalkParams{{NameLbl: "public", Retries: 2}}, } res := args.Convert() @@ -129,8 +131,8 @@ func TestConvertConfig(t *testing.T) { func TestConvertConfigWithInlineConfig(t *testing.T) { args := Arguments{ ConfigStruct: config.Config{Modules: map[string]*config.Module{"if_mib": {Walk: []string{"1.3.6.1.2.1.2"}}}}, - Targets: TargetBlock{{Name: "network_switch_1", Target: "192.168.1.2", Module: "if_mib"}}, - WalkParams: WalkParams{{Name: "public", Retries: 2}}, + Targets: TargetBlock{{NameLbl: "network_switch_1", Target: "192.168.1.2", Module: "if_mib"}}, + WalkParams: WalkParams{{NameLbl: "public", Retries: 2}}, } res := args.Convert() @@ -141,7 +143,7 @@ func TestConvertConfigWithInlineConfig(t *testing.T) { func TestConvertTargets(t *testing.T) { targets := TargetBlock{{ - Name: "network_switch_1", + NameLbl: "network_switch_1", Target: "192.168.1.2", Module: "if_mib", Auth: "public_v2", @@ -252,6 +254,81 @@ func TestValidateTargetMissingAddress(t *testing.T) { require.ErrorContains(t, err, "all targets must have an `address`") } +func TestUseBothLabelAndNameParam(t *testing.T) { + testCfg := []struct { + name string + cfg string + err string + }{ + { + name: "target_different_label_and_name", + cfg: ` + target "t2" { + name = "t5" + address = "192.168.1.3" + module = "mikrotik" + walk_params = "private" + }`, + err: "the label of a `target` block and the `name` argument are mutually exclusive", + }, + { + name: "target_same_label_and_name", + cfg: ` + target "t2" { + name = "t2" + address = "192.168.1.3" + module = "mikrotik" + walk_params = "private" + }`, + err: "the label of a `target` block and the `name` argument are mutually exclusive", + }, + { + name: "target_neither_label_nor_name", + cfg: ` + target { + address = "192.168.1.3" + module = "mikrotik" + walk_params = "private" + }`, + err: "either the label of a `target` block or the `name` argument must be set", + }, + { + name: "walk_param_different_label_and_name", + cfg: ` + walk_param "w3" { + name = "w5" + retries = 2 + }`, + err: "the label of a `walk_param` block and the `name` argument are mutually exclusive", + }, + { + name: "walk_param_same_label_and_name", + cfg: ` + walk_param "w3" { + name = "w3" + retries = 2 + }`, + err: "the label of a `walk_param` block and the `name` argument are mutually exclusive", + }, + { + name: "walk_neither_label_nor_name", + cfg: ` + walk_param { + retries = 2 + }`, + err: "either the label of a `walk_param` block or the `name` argument must be set", + }, + } + + for _, tt := range testCfg { + t.Run(tt.name, func(t *testing.T) { + var args Arguments + err := syntax.Unmarshal([]byte(tt.cfg), &args) + require.ErrorContains(t, err, tt.err) + }) + } +} + func TestValidateTargetsMutualExclusivity(t *testing.T) { alloyCfg := ` config_file = "modules.yml" @@ -278,7 +355,7 @@ func TestValidateTargetsMutualExclusivity(t *testing.T) { func TestConvertWalkParams(t *testing.T) { retries := 3 walkParams := WalkParams{{ - Name: "public", + NameLbl: "public", MaxRepetitions: uint32(10), Retries: retries, Timeout: time.Duration(5), @@ -296,9 +373,9 @@ func TestConvertWalkParams(t *testing.T) { func TestBuildSNMPTargets(t *testing.T) { baseArgs := Arguments{ ConfigFile: "modules.yml", - Targets: TargetBlock{{Name: "network_switch_1", Target: "192.168.1.2", Module: "if_mib", + Targets: TargetBlock{{NameLbl: "network_switch_1", Target: "192.168.1.2", Module: "if_mib", WalkParams: "public", Auth: "public_v2"}}, - WalkParams: WalkParams{{Name: "public", Retries: 2}}, + WalkParams: WalkParams{{NameLbl: "public", Retries: 2}}, } baseTarget := discovery.Target{ model.SchemeLabel: "http", @@ -351,13 +428,13 @@ func TestUnmarshalAlloyWithInlineConfig(t *testing.T) { require.Equal(t, args.ConfigStruct.Auths["public_v1"].Version, 1) require.Equal(t, 2, len(args.Targets)) - require.Contains(t, "network_switch_1", args.Targets[0].Name) + require.Contains(t, "network_switch_1", args.Targets[0].Name()) require.Contains(t, "192.168.1.2", args.Targets[0].Target) require.Contains(t, "if_mib", args.Targets[0].Module) require.Contains(t, "public", args.Targets[0].WalkParams) require.Contains(t, "public_v1", args.Targets[0].Auth) - require.Contains(t, "network_router_2", args.Targets[1].Name) + require.Contains(t, "network_router_2", args.Targets[1].Name()) require.Contains(t, "192.168.1.3", args.Targets[1].Target) require.Contains(t, "if_mib", args.Targets[1].Module) require.Contains(t, "private", args.Targets[1].WalkParams) diff --git a/internal/converter/internal/staticconvert/internal/build/snmp_exporter.go b/internal/converter/internal/staticconvert/internal/build/snmp_exporter.go index 44557042e6..40716100c9 100644 --- a/internal/converter/internal/staticconvert/internal/build/snmp_exporter.go +++ b/internal/converter/internal/staticconvert/internal/build/snmp_exporter.go @@ -3,7 +3,6 @@ package build import ( "github.com/grafana/alloy/internal/component/discovery" "github.com/grafana/alloy/internal/component/prometheus/exporter/snmp" - "github.com/grafana/alloy/internal/converter/internal/common" "github.com/grafana/alloy/internal/static/integrations/snmp_exporter" snmp_exporter_v2 "github.com/grafana/alloy/internal/static/integrations/v2/snmp_exporter" "github.com/grafana/alloy/syntax/alloytypes" @@ -37,7 +36,7 @@ func toSnmpExporter(config *snmp_exporter.Config) *snmp.Arguments { } walkParams[index] = snmp.WalkParam{ - Name: common.SanitizeIdentifierPanics(name), + NameArg: name, MaxRepetitions: p.MaxRepetitions, Retries: retries, Timeout: p.Timeout, @@ -86,7 +85,7 @@ func toSnmpExporterV2(config *snmp_exporter_v2.Config) *snmp.Arguments { } walkParams[index] = snmp.WalkParam{ - Name: common.SanitizeIdentifierPanics(name), + NameArg: name, MaxRepetitions: p.MaxRepetitions, Retries: retries, Timeout: p.Timeout, diff --git a/syntax/internal/syntaxtags/syntaxtags.go b/syntax/internal/syntaxtags/syntaxtags.go index d8262bbb84..67ab71a92e 100644 --- a/syntax/internal/syntaxtags/syntaxtags.go +++ b/syntax/internal/syntaxtags/syntaxtags.go @@ -277,6 +277,8 @@ func parseFlags(input string) (f Flags, ok bool) { f |= FlagEnum | FlagOptional case "label": f |= FlagLabel + case "label,optional": + f |= FlagLabel | FlagOptional case "squash": f |= FlagSquash default: diff --git a/syntax/internal/syntaxtags/syntaxtags_test.go b/syntax/internal/syntaxtags/syntaxtags_test.go index e2ecc8ff02..3cc706b261 100644 --- a/syntax/internal/syntaxtags/syntaxtags_test.go +++ b/syntax/internal/syntaxtags/syntaxtags_test.go @@ -37,6 +37,20 @@ func Test_Get(t *testing.T) { require.Equal(t, expect, fs) } +func Test_GetOptionalLabel(t *testing.T) { + type Struct struct { + Label string `alloy:",label,optional"` + } + + fs := syntaxtags.Get(reflect.TypeOf(Struct{})) + + expect := []syntaxtags.Field{ + {[]string{""}, []int{0}, syntaxtags.FlagLabel | syntaxtags.FlagOptional}, + } + + require.Equal(t, expect, fs) +} + func TestEmbedded(t *testing.T) { type InnerStruct struct { InnerField1 string `alloy:"inner_field_1,attr"` @@ -179,4 +193,13 @@ func Test_Get_Panics(t *testing.T) { expect := `syntax: label field already used by syntaxtags_test.Struct.Label2` expectPanic(t, expect, Struct{}) }) + + t.Run("Only one label field may exist (with an optional label)", func(t *testing.T) { + type Struct struct { + Label1 string `alloy:",label"` + Label2 string `alloy:",label,optional"` + } + expect := `syntax: label field already used by syntaxtags_test.Struct.Label2` + expectPanic(t, expect, Struct{}) + }) } diff --git a/syntax/vm/vm.go b/syntax/vm/vm.go index 2df052b92b..d38629a7dd 100644 --- a/syntax/vm/vm.go +++ b/syntax/vm/vm.go @@ -247,7 +247,7 @@ func (vm *Evaluator) evaluateBlockLabel(node *ast.BlockStmt, tfs []syntaxtags.Fi // the name. We might be able to clean this up in the future by extending // ValueError to have an explicit position. switch { - case node.Label == "" && foundField: // No user label, but struct expects one + case node.Label == "" && foundField && !labelField.IsOptional(): // No user label, but struct expects one return diag.Diagnostic{ Severity: diag.SeverityLevelError, StartPos: node.NamePos.Position(), diff --git a/syntax/vm/vm_block_test.go b/syntax/vm/vm_block_test.go index d911f3a3f1..0787c73b5a 100644 --- a/syntax/vm/vm_block_test.go +++ b/syntax/vm/vm_block_test.go @@ -640,6 +640,45 @@ func TestVM_Block_Label(t *testing.T) { err := eval.Evaluate(nil, &block{}) require.EqualError(t, err, `1:1: block "some_block" requires non-empty label`) }) + + t.Run("Block may have an optional label", func(t *testing.T) { + type block struct { + Label string `alloy:",label,optional"` + } + + input := `some_block "asdf" {}` + eval := vm.New(parseBlock(t, input)) + + var actual block + require.NoError(t, eval.Evaluate(nil, &actual)) + require.Equal(t, "asdf", actual.Label) + }) + + t.Run("Block may have an optional label with explicit empty value", func(t *testing.T) { + type block struct { + Label string `alloy:",label,optional"` + } + + input := `some_block "" {}` + eval := vm.New(parseBlock(t, input)) + + var actual block + require.NoError(t, eval.Evaluate(nil, &actual)) + require.Equal(t, "", actual.Label) + }) + + t.Run("Block may have an optional label with no value", func(t *testing.T) { + type block struct { + Label string `alloy:",label,optional"` + } + + input := `some_block {}` + eval := vm.New(parseBlock(t, input)) + + var actual block + require.NoError(t, eval.Evaluate(nil, &actual)) + require.Equal(t, "", actual.Label) + }) } func TestVM_Block_Unmarshaler(t *testing.T) {