Skip to content

Commit e5207d9

Browse files
committed
Address maintainer feedback: add validations and remove changelog
- Remove changelog entry (mdatagen is internal tool) - Add validation: feature gates must be sorted by ID - Add validation: each feature gate must have reference_url - Add validation: ID must match regex ^[0-9a-zA-Z.]*$ - Add comprehensive tests for all validations - Revert unrelated formatting changes to package_test.go.tmpl
1 parent 2bdcaed commit e5207d9

File tree

4 files changed

+68
-34
lines changed

4 files changed

+68
-34
lines changed

.chloggen/mdatagen-featuregates-docs.yaml

Lines changed: 0 additions & 28 deletions
This file was deleted.

cmd/mdatagen/internal/metadata.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,14 @@ func validateEvents(events map[EventName]Event, attributes map[AttributeName]Att
280280
func (md *Metadata) validateFeatureGates() error {
281281
var errs error
282282
seen := make(map[FeatureGateID]bool)
283+
idRegexp := regexp.MustCompile(`^[0-9a-zA-Z.]*$`)
284+
285+
// Validate that feature gates are sorted by ID
286+
if !slices.IsSortedFunc(md.FeatureGates, func(a, b FeatureGate) int {
287+
return strings.Compare(string(a.ID), string(b.ID))
288+
}) {
289+
errs = errors.Join(errs, errors.New("feature gates must be sorted by ID"))
290+
}
283291

284292
for i, gate := range md.FeatureGates {
285293
// Validate gate ID is not empty
@@ -288,6 +296,11 @@ func (md *Metadata) validateFeatureGates() error {
288296
continue
289297
}
290298

299+
// Validate ID follows the allowed character pattern
300+
if !idRegexp.MatchString(string(gate.ID)) {
301+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": ID contains invalid characters, must match ^[0-9a-zA-Z.]*$`, gate.ID))
302+
}
303+
291304
// Check for duplicate IDs
292305
if seen[gate.ID] {
293306
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": duplicate ID`, gate.ID))
@@ -300,6 +313,11 @@ func (md *Metadata) validateFeatureGates() error {
300313
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": description is required`, gate.ID))
301314
}
302315

316+
// Validate that each feature gate has a reference link
317+
if gate.ReferenceURL == "" {
318+
errs = errors.Join(errs, fmt.Errorf(`feature gate "%v": reference_url is required`, gate.ID))
319+
}
320+
303321
// Validate stage is one of the allowed values
304322
validStages := map[FeatureGateStage]bool{
305323
FeatureGateStageAlpha: true,

cmd/mdatagen/internal/metadata_test.go

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,12 @@ func TestValidateFeatureGates(t *testing.T) {
343343
{
344344
name: "valid stable gate with to_version",
345345
featureGate: FeatureGate{
346-
ID: "component.stable",
347-
Description: "Stable feature gate",
348-
Stage: FeatureGateStageStable,
349-
FromVersion: "v0.90.0",
350-
ToVersion: "v0.95.0",
346+
ID: "component.stable",
347+
Description: "Stable feature gate",
348+
Stage: FeatureGateStageStable,
349+
FromVersion: "v0.90.0",
350+
ToVersion: "v0.95.0",
351+
ReferenceURL: "https://example.com",
351352
},
352353
},
353354
{
@@ -410,6 +411,27 @@ func TestValidateFeatureGates(t *testing.T) {
410411
},
411412
wantErr: `to_version is required for deprecated stage gates`,
412413
},
414+
{
415+
name: "missing reference_url",
416+
featureGate: FeatureGate{
417+
ID: "component.feature",
418+
Description: "Test feature",
419+
Stage: FeatureGateStageAlpha,
420+
FromVersion: "v0.100.0",
421+
},
422+
wantErr: `reference_url is required`,
423+
},
424+
{
425+
name: "invalid characters in ID",
426+
featureGate: FeatureGate{
427+
ID: "component.feature@invalid",
428+
Description: "Test feature",
429+
Stage: FeatureGateStageAlpha,
430+
FromVersion: "v0.100.0",
431+
ReferenceURL: "https://example.com",
432+
},
433+
wantErr: `ID contains invalid characters`,
434+
},
413435
}
414436

415437
for _, tt := range tests {
@@ -461,3 +483,25 @@ func TestValidateFeatureGatesDuplicateID(t *testing.T) {
461483
require.Error(t, err)
462484
assert.ErrorContains(t, err, "duplicate ID")
463485
}
486+
487+
func TestValidateFeatureGatesNotSorted(t *testing.T) {
488+
md := &Metadata{
489+
FeatureGates: []FeatureGate{
490+
{
491+
ID: "component.zebra",
492+
Description: "Test feature",
493+
Stage: FeatureGateStageAlpha,
494+
ReferenceURL: "https://example.com",
495+
},
496+
{
497+
ID: "component.alpha",
498+
Description: "Another feature",
499+
Stage: FeatureGateStageAlpha,
500+
ReferenceURL: "https://example.com",
501+
},
502+
},
503+
}
504+
err := md.validateFeatureGates()
505+
require.Error(t, err)
506+
assert.ErrorContains(t, err, "feature gates must be sorted by ID")
507+
}

cmd/mdatagen/internal/templates/package_test.go.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
"os"
88
{{- end }}
99
"testing"
10-
{{- if not .Tests.GoLeak.Skip }}
1110

11+
{{- if not .Tests.GoLeak.Skip }}
1212
"go.uber.org/goleak"
1313
{{- end }}
1414
)

0 commit comments

Comments
 (0)