Skip to content

Commit 085ec78

Browse files
j10czarSupam
authored andcommitted
Implement tracking-field decoder + duplicate artifact name detection
1 parent 5eb0064 commit 085ec78

14 files changed

Lines changed: 821 additions & 5 deletions

.generator-v2/go.mod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ toolchain go1.26.1
66

77
require (
88
github.com/pb33f/libopenapi v0.37.2
9+
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
910
github.com/spf13/cobra v1.10.2
11+
go.yaml.in/yaml/v4 v4.0.0-rc.4
1012
)
1113

1214
require (
@@ -16,6 +18,6 @@ require (
1618
github.com/pb33f/jsonpath v0.8.2 // indirect
1719
github.com/pb33f/ordered-map/v2 v2.3.1 // indirect
1820
github.com/spf13/pflag v1.0.9 // indirect
19-
go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect
2021
golang.org/x/sync v0.20.0 // indirect
22+
golang.org/x/text v0.14.0 // indirect
2123
)

.generator-v2/go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ github.com/buger/jsonparser v1.1.2/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx2
55
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
66
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
77
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
8+
github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
9+
github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
810
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
911
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
1012
github.com/pb33f/jsonpath v0.8.2 h1:Ou4C7zjYClBm97dfZjDCjdZGusJoynv/vrtiEKNfj2Y=
@@ -16,6 +18,8 @@ github.com/pb33f/ordered-map/v2 v2.3.1/go.mod h1:qxFQgd0PkVUtOMCkTapqotNgzRhMPL7
1618
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
1719
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
1820
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
21+
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ=
22+
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
1923
github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU=
2024
github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4=
2125
github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY=
@@ -27,6 +31,8 @@ go.yaml.in/yaml/v4 v4.0.0-rc.4 h1:UP4+v6fFrBIb1l934bDl//mmnoIZEDK0idg1+AIvX5U=
2731
go.yaml.in/yaml/v4 v4.0.0-rc.4/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0=
2832
golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
2933
golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
34+
golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
35+
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
3036
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
3137
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
3238
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

.generator-v2/internal/cli/generate.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ func newGenerateCmd(flags *globalFlags) *cobra.Command {
1414
Use: "generate",
1515
Short: "Generate Terraform artifacts from the OpenAPI spec",
1616
RunE: func(cmd *cobra.Command, args []string) error {
17-
spec, err := parser.LoadSpec(flags.spec, parser.WithMaxDepth(flags.maxDepth))
17+
spec, err := parser.LoadSpec(flags.spec,
18+
parser.WithMaxDepth(flags.maxDepth),
19+
parser.WithTrackingFieldName(flags.trackingField))
1820
if err != nil {
1921
return err
2022
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Package contracts holds the machine-readable contracts the generator
2+
// validates against. The files are embedded so the tfgen binary is
3+
// self-contained and never depends on its working directory at runtime.
4+
//
5+
// New contracts (e.g. a CLI-flag doc or a run-report schema) are added here as
6+
// another //go:embed directive plus an exported var.
7+
package contracts
8+
9+
import _ "embed"
10+
11+
// TrackingFieldSchema is the embedded JSON Schema (draft 2020-12) for the
12+
// x-datadog-tf-generator OpenAPI vendor extension. parser.DecodeTracking
13+
// compiles it once and validates every extension against it.
14+
//
15+
//go:embed tracking-field.schema.json
16+
var TrackingFieldSchema []byte
17+
18+
// TrackingFieldSchemaID is the schema's canonical $id — the URL it is
19+
// registered and compiled under by the validator.
20+
const TrackingFieldSchemaID = "https://datadog.github.io/terraform-provider-datadog/contracts/tracking-field.schema.json"
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
{
2+
"$schema": "https://json-schema.org/draft/2020-12/schema",
3+
"$id": "https://datadog.github.io/terraform-provider-datadog/contracts/tracking-field.schema.json",
4+
"title": "x-datadog-tf-generator",
5+
"description": "OpenAPI 3.0 vendor extension that opts an operation in to Datadog Terraform provider generation.",
6+
"type": "object",
7+
"additionalProperties": false,
8+
"required": ["artifact_kind", "artifact_name"],
9+
"properties": {
10+
"artifact_kind": {
11+
"type": "string",
12+
"enum": ["resource", "data_source"],
13+
"description": "Whether this operation should be exposed as a Terraform resource (with full CRUD lifecycle) or a Terraform data source (read-only)."
14+
},
15+
"artifact_name": {
16+
"type": "string",
17+
"pattern": "^[a-z][a-z0-9_]*$",
18+
"minLength": 1,
19+
"maxLength": 64,
20+
"description": "Terraform-facing artifact name without the 'datadog_' prefix. Must be lowercase snake_case. Must be unique across the entire spec."
21+
},
22+
"group": {
23+
"type": "object",
24+
"description": "Declares which OpenAPI operations form the C/R/U/D quadruple. May reference operations by their operationId.",
25+
"additionalProperties": false,
26+
"properties": {
27+
"create": { "type": "string", "description": "operationId of the Create endpoint." },
28+
"read": { "type": "string", "description": "operationId of the Read endpoint." },
29+
"update": { "type": "string", "description": "operationId of the Update endpoint. May be omitted; the generator will then mark all attributes ForceNew per the missing-CRUD edge case in the spec." },
30+
"delete": { "type": "string", "description": "operationId of the Delete endpoint." }
31+
},
32+
"required": ["read"]
33+
},
34+
"id_strategy": {
35+
"type": "string",
36+
"enum": ["data.id", "data.attributes.id", "data.attributes.uuid", "header.location"],
37+
"default": "data.id",
38+
"description": "How to derive the Terraform resource ID from the API response on Create/Read."
39+
},
40+
"sensitive": {
41+
"type": "boolean",
42+
"default": false,
43+
"description": "When attached to a Schema Object, marks the attribute as Terraform-sensitive (suppressed in plan output)."
44+
},
45+
"skip": {
46+
"type": "boolean",
47+
"default": false,
48+
"description": "Explicitly disable generation for this operation while keeping the annotation in place. Equivalent to removing the extension, but documented in-spec so reviewers see the choice."
49+
}
50+
},
51+
"examples": [
52+
{
53+
"artifact_kind": "data_source",
54+
"artifact_name": "team",
55+
"group":{
56+
"read": "GetTeam"
57+
}
58+
},
59+
{
60+
"artifact_kind": "resource",
61+
"artifact_name": "incident_type",
62+
"group": {
63+
"create": "CreateIncidentType",
64+
"read": "GetIncidentType",
65+
"update": "UpdateIncidentType",
66+
"delete": "DeleteIncidentType"
67+
},
68+
"id_strategy": "data.id"
69+
}
70+
]
71+
}
72+
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package parser
2+
3+
import (
4+
"fmt"
5+
"sort"
6+
"strings"
7+
8+
"github.com/terraform-providers/terraform-provider-datadog/generator/internal/model"
9+
)
10+
11+
// OperationLocation identifies one operation participating in an artifact_name
12+
// collision.
13+
type OperationLocation struct {
14+
Path string
15+
Method string
16+
OperationId string
17+
}
18+
19+
// ArtifactNameCollision records every operation that declared a given
20+
// artifact_name, when more than one did.
21+
type ArtifactNameCollision struct {
22+
Name string
23+
Sources []OperationLocation
24+
}
25+
26+
// DuplicateArtifactNameError aggregates every artifact_name collision found
27+
// across the spec into a single error, naming all source locations for each.
28+
// Collisions are sorted by name and their sources by (path, method), so the
29+
// message is deterministic regardless of input order.
30+
type DuplicateArtifactNameError struct {
31+
Collisions []ArtifactNameCollision
32+
}
33+
34+
func (e *DuplicateArtifactNameError) Error() string {
35+
var b strings.Builder
36+
b.WriteString("parser: duplicate artifact_name across operations:")
37+
for _, c := range e.Collisions {
38+
fmt.Fprintf(&b, "\n %q declared by:", c.Name)
39+
for _, s := range c.Sources {
40+
fmt.Fprintf(&b, "\n - %s %s (operationId %q)", s.Method, s.Path, s.OperationId)
41+
}
42+
}
43+
return b.String()
44+
}
45+
46+
// CheckDuplicateArtifactNames reports every artifact_name claimed by more than
47+
// one operation. Operations without tracking metadata are ignored (they
48+
// generate no artifact and so cannot collide). It returns a single aggregated
49+
// *DuplicateArtifactNameError naming every collision and all of its sources, or
50+
// nil when all names are unique.
51+
func CheckDuplicateArtifactNames(spec *model.Spec) error {
52+
byName := make(map[string][]OperationLocation)
53+
for _, op := range spec.Operations {
54+
if op == nil || op.Tracking == nil {
55+
continue
56+
}
57+
byName[op.Tracking.ArtifactName] = append(byName[op.Tracking.ArtifactName], OperationLocation{
58+
Path: op.Path,
59+
Method: op.Method,
60+
OperationId: op.OperationId,
61+
})
62+
}
63+
64+
var collisions []ArtifactNameCollision
65+
for name, locs := range byName {
66+
if len(locs) < 2 {
67+
continue
68+
}
69+
sort.Slice(locs, func(i, j int) bool {
70+
if locs[i].Path != locs[j].Path {
71+
return locs[i].Path < locs[j].Path
72+
}
73+
return locs[i].Method < locs[j].Method
74+
})
75+
collisions = append(collisions, ArtifactNameCollision{Name: name, Sources: locs})
76+
}
77+
if len(collisions) == 0 {
78+
return nil
79+
}
80+
sort.Slice(collisions, func(i, j int) bool { return collisions[i].Name < collisions[j].Name })
81+
return &DuplicateArtifactNameError{Collisions: collisions}
82+
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package parser
2+
3+
import (
4+
"errors"
5+
"strings"
6+
"testing"
7+
8+
"github.com/terraform-providers/terraform-provider-datadog/generator/internal/model"
9+
)
10+
11+
// trackedOp builds a model.Operation carrying tracking metadata for the given
12+
// artifact name. The kind is irrelevant to duplicate detection.
13+
func trackedOp(path, method, operationId, artifactName string) *model.Operation {
14+
return &model.Operation{
15+
Path: path,
16+
Method: method,
17+
OperationId: operationId,
18+
Tracking: &model.TrackingFieldMetadata{
19+
ArtifactKind: model.ArtifactKindResource,
20+
ArtifactName: artifactName,
21+
},
22+
}
23+
}
24+
25+
func TestCheckDuplicateArtifactNamesUnique(t *testing.T) {
26+
spec := &model.Spec{Operations: []*model.Operation{
27+
trackedOp("/a", "GET", "GetA", "alpha"),
28+
trackedOp("/b", "GET", "GetB", "beta"),
29+
trackedOp("/c", "GET", "GetC", "gamma"),
30+
}}
31+
if err := CheckDuplicateArtifactNames(spec); err != nil {
32+
t.Fatalf("expected nil, got %v", err)
33+
}
34+
}
35+
36+
func TestCheckDuplicateArtifactNamesIgnoresUntracked(t *testing.T) {
37+
spec := &model.Spec{Operations: []*model.Operation{
38+
trackedOp("/a", "GET", "GetA", "alpha"),
39+
{Path: "/health", Method: "GET", OperationId: "GetHealth"}, // Tracking nil
40+
nil, // defensive: nil entries are skipped
41+
trackedOp("/b", "GET", "GetB", "beta"),
42+
}}
43+
if err := CheckDuplicateArtifactNames(spec); err != nil {
44+
t.Fatalf("expected nil, got %v", err)
45+
}
46+
}
47+
48+
func TestCheckDuplicateArtifactNamesSingleCollision(t *testing.T) {
49+
spec := &model.Spec{Operations: []*model.Operation{
50+
trackedOp("/teams", "GET", "ListTeams", "team"),
51+
trackedOp("/teams/{id}", "GET", "GetTeam", "team"),
52+
}}
53+
err := CheckDuplicateArtifactNames(spec)
54+
var dup *DuplicateArtifactNameError
55+
if !errors.As(err, &dup) {
56+
t.Fatalf("error %v (%T) is not a *DuplicateArtifactNameError", err, err)
57+
}
58+
if len(dup.Collisions) != 1 {
59+
t.Fatalf("got %d collisions, want 1", len(dup.Collisions))
60+
}
61+
if len(dup.Collisions[0].Sources) != 2 {
62+
t.Fatalf("got %d sources, want 2", len(dup.Collisions[0].Sources))
63+
}
64+
msg := dup.Error()
65+
for _, want := range []string{"team", "ListTeams", "GetTeam", "/teams", "/teams/{id}"} {
66+
if !strings.Contains(msg, want) {
67+
t.Errorf("error message missing %q:\n%s", want, msg)
68+
}
69+
}
70+
}
71+
72+
func TestCheckDuplicateArtifactNamesListsAllSources(t *testing.T) {
73+
spec := &model.Spec{Operations: []*model.Operation{
74+
trackedOp("/teams", "GET", "ListTeams", "team"),
75+
trackedOp("/teams/{id}", "GET", "GetTeam", "team"),
76+
trackedOp("/teams/search", "POST", "SearchTeams", "team"),
77+
}}
78+
err := CheckDuplicateArtifactNames(spec)
79+
var dup *DuplicateArtifactNameError
80+
if !errors.As(err, &dup) {
81+
t.Fatalf("error %v (%T) is not a *DuplicateArtifactNameError", err, err)
82+
}
83+
if n := len(dup.Collisions[0].Sources); n != 3 {
84+
t.Fatalf("got %d sources, want all 3 listed", n)
85+
}
86+
for _, want := range []string{"ListTeams", "GetTeam", "SearchTeams"} {
87+
if !strings.Contains(dup.Error(), want) {
88+
t.Errorf("error message missing source %q", want)
89+
}
90+
}
91+
}
92+
93+
func TestCheckDuplicateArtifactNamesMultipleCollisionsSortedByName(t *testing.T) {
94+
spec := &model.Spec{Operations: []*model.Operation{
95+
trackedOp("/z", "GET", "GetZ1", "zeta"),
96+
trackedOp("/z2", "GET", "GetZ2", "zeta"),
97+
trackedOp("/a", "GET", "GetA1", "alpha"),
98+
trackedOp("/a2", "GET", "GetA2", "alpha"),
99+
}}
100+
err := CheckDuplicateArtifactNames(spec)
101+
var dup *DuplicateArtifactNameError
102+
if !errors.As(err, &dup) {
103+
t.Fatalf("error %v (%T) is not a *DuplicateArtifactNameError", err, err)
104+
}
105+
if len(dup.Collisions) != 2 {
106+
t.Fatalf("got %d collisions, want 2", len(dup.Collisions))
107+
}
108+
if dup.Collisions[0].Name != "alpha" || dup.Collisions[1].Name != "zeta" {
109+
t.Errorf("collisions not sorted by name: %q then %q", dup.Collisions[0].Name, dup.Collisions[1].Name)
110+
}
111+
}
112+
113+
func TestCheckDuplicateArtifactNamesDeterministic(t *testing.T) {
114+
// Same collisions, different declaration orders, must yield identical output.
115+
build := func(ops ...*model.Operation) *model.Spec { return &model.Spec{Operations: ops} }
116+
a := build(
117+
trackedOp("/teams", "GET", "ListTeams", "team"),
118+
trackedOp("/teams/{id}", "GET", "GetTeam", "team"),
119+
trackedOp("/users", "GET", "ListUsers", "user"),
120+
trackedOp("/users/{id}", "GET", "GetUser", "user"),
121+
)
122+
b := build(
123+
trackedOp("/users/{id}", "GET", "GetUser", "user"),
124+
trackedOp("/teams/{id}", "GET", "GetTeam", "team"),
125+
trackedOp("/users", "GET", "ListUsers", "user"),
126+
trackedOp("/teams", "GET", "ListTeams", "team"),
127+
)
128+
errA, errB := CheckDuplicateArtifactNames(a), CheckDuplicateArtifactNames(b)
129+
if errA == nil || errB == nil {
130+
t.Fatal("expected duplicate errors from both specs")
131+
}
132+
if errA.Error() != errB.Error() {
133+
t.Errorf("non-deterministic output:\nA:\n%s\nB:\n%s", errA.Error(), errB.Error())
134+
}
135+
}

0 commit comments

Comments
 (0)